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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 17, 2024, 15:05 UTC
Message-ID: <xmqqh6g0av28.fsf@gitster.g>
In-Reply-To: <1d0e98cb-78a4-40d0-9bfe-390a3a30aad8@gmail.com>

phillip.wood123@gmail.com 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.

Show 8 quoted lines

> 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.