On 17/04/2024 16:05, Junio C Hamano wrote:
phillip.wood123@xxxxxxxxx writes:
I tend to associate "option" with a command-line argument, not
interactive input to a program.
"git add --help" is a bit mixed. The choices offered by "git add
-i" are called "subcommand" (see "INTERACTIVE MODE" section), but
the choices you give to the prompt "patch" subcommand gives you are
presented with "You can select one of the following options and type
return". So "option" is not too wrong, even though it is a word
used in other contexts as well. I am OK with "option", but if I
were adding this new error message, I probably would have said
"unknown command".
I think "unknown command" is a good suggestion, I take your point about
"unknown key" not being so clear for users who do not use single-key input.
Best Wishes
Phillip
In any case, whether you said option, command, or key , it is so
obvious from the context that we could even say "error: 'W' not
known, use '?' for help" without any noun there, so it would not
matter too much which noun you pick.
I'd still avoid "key", though, because to those who do not do
single-key input, myself included, it does not match their user
experience, and it is even more so if they forgot or do not even
know that they could choose to use single-key input.
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.
I do agree with these three points, but I do not have a strong
opinion on the new test that was added by the patch when judging
with them used as a yardstick.
Thanks.
