From: phillip.wood123@gmail.com Date: Thu, 18 Apr 2024 15:11:33 GMT Subject: Re: [PATCH] add-patch: response to invalid option Message-ID: <52102ebc-ae57-4000-9990-7def910ba254@gmail.com> In-Reply-To: On 17/04/2024 16:05, Junio C Hamano wrote: > 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 > 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.