Hi Rubén
On 16/04/2024 20:24, Rubén Justo wrote:
On Tue, Apr 16, 2024 at 10:41:32AM +0100, phillip.wood123@xxxxxxxxx wrote:
On 15/04/2024 20:00, Rubén Justo wrote:
+ } else {
+ err(s, _("Unknown option '%s' (use '?' for help)"),
As this is an interactive program I think "Unknown key" would be clearer.
Maybe you have "interactive.singleKey" set to "true"?
Perhaps "key" refers more to the key pressed by the user. My impression
is that "option" have a clearer and wider audience.
I tend to associate "option" with a command-line argument, not
interactive input to a program.
Something like this (which also adds coverage for '?' and 'p')
I know we don't have a test for the help. I noticed this in my other
series. And I decided not to include a test for it then. Maybe in this
series it makes sense.
However, I would prefer to keep a test only on the new error message.
Why? This commit makes three changes
- it stops printing the help when the user presses an unknown key
- it starts treating '?' differently to an unknown key
- it starts printing an error message when the user presses an unknown
key
The test you are proposing only tests the last of these changes. We
should be aiming to write tests that (a) verify all of the changes
introduced by a commit (b) are likely to detect regressions to those
changes (c) are reasonably efficient, for example if it is possible to
test more than one key with a single "add -p" process we should do so.
As this is an interactive program I have a strong preference for testing
what the user sees printed to their screen, not just what happens to
come out on stderr.
Thanks
Phillip
