Subject: Re: [PATCH] add-patch: response to invalid option

From: Junio C Hamano <[email-masked]>
Date: Wed, 17 Apr 2024 08:05:35 -0700

[email-masked] 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".

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.