Re: [PATCH] add-patch: response to invalid option
- From
- phillip.wood123@gmail.com <phillip.wood123@gmail.com>
- Date
- Apr 18, 2024, 15:11 UTC
- Message-ID
- <52102ebc-ae57-4000-9990-7def910ba254@gmail.com>
- In-Reply-To
- <xmqqh6g0av28.fsf@gitster.g>
On 17/04/2024 16:05, Junio C Hamano wrote:
Show 13 quoted lines
> 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".
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
Show 24 quoted lines
> 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.