git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] add-patch: enforce only one-letter response to prompts

From
DSDragan Simic <dsimic@manjaro.org>
Date
May 22, 2024, 06:40 UTC
Message-ID
<fbb9c7d3e7c2129bc1526dfa5a8eca0c@manjaro.org>
In-Reply-To
<xmqqh6eqiwgf.fsf@gitster.g>
Hello Junio,
Please see my comments below.
On 2024-05-22 01:20, Junio C Hamano wrote:
> In an "git add -p" session, especially when we are not using the
s/In an/In a/
> single-char mode, we may see 'qa' as a response to a prompt
Perhaps s/single-char/single-character/
Show 60 quoted lines
> 
>   (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?
> 
> and then just do the 'q' thing (i.e. quit the session), ignoring
> everything other than the first byte.
> 
> If 'q' and 'a' are next to each other on the user's keyboard, there
> is a plausible chance that we see 'qa' when the user who wanted to
> say 'a' fat-fingered and we ended up doing the 'q' thing instead.
> 
> As we didn't think of a good reason during the review discussion why
> we want to accept excess letters only to ignore them, it appears to
> be a safe change to simply reject input that is longer than just one
> byte.
> 
> The two exceptions are the 'g' command that takes a hunk number, and
> the '/' command that takes a regular expression.  They has to be
> accompanied by their operands (this makes me wonder how users who
> set the interactive.singlekey configuration feed these operands---it
> turns out that we notice there is no operand and give them another
> chance to type the operand separately, without using single key
> input this time), so we accept a string that is more than one byte
> long.
> 
> Keep the "use only the first byte, downcased" behaviour when we ask
> yes/no question, though.  Neither on Qwerty or on Dvorak, 'y' and
> 'n' are not close to each other.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  * This version fixes the breakage in t3701 where we exercise the
>    '/' command.  Further code inspection reveals that 'g' also needs
>    to be special cased.
> 
>    The previous iteration was <xmqqr0dvb1sh.fsf_-_@gitster.g>.
> 
>  add-patch.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/add-patch.c b/add-patch.c
> index 79eda168eb..a6c3367d59 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1228,6 +1228,7 @@ static int prompt_yesno(struct add_p_state *s,
> const char *prompt)
>  		fflush(stdout);
>  		if (read_single_character(s) == EOF)
>  			return -1;
> +		/* do not limit to 1-byte input to allow 'no' etc. */
>  		switch (tolower(s->answer.buf[0])) {
>  		case 'n': return 0;
>  		case 'y': return 1;
> @@ -1506,6 +1507,12 @@ static int patch_update_file(struct add_p_state 
> *s,
>  		if (!s->answer.len)
>  			continue;
>  		ch = tolower(s->answer.buf[0]);
> +
> +		/* 'g' takes a hunk number, '/' takes a regexp */
> +		if (1 < s->answer.len && (ch != 'g' && ch != '/')) {

To me, "s->answer.len > 1" would be much more readable, and I was surprised a bit to see the flipped variant. This made me curious; would you, please, let me know why do you prefer this form?

Show 6 quoted lines
> +			error(_("only one letter is expected, got '%s'"), s->answer.buf);
> +			continue;
> +		}
>  		if (ch == 'y') {
>  			hunk->use = USE_HUNK;
>  soft_increment:

The patch is looking good to me, and I find it good that it improves the strictness of the user input, which should also improve the overall user experience.

Previous: Junio C HamanoNext: Junio C Hamano
Message 10 of 21 in “add-patch: response to unknown command”
  1. add-patch: response to unknown commandRubén Justo, May 21, 2024
  2. Patrick SteinhardtMay 21, 2024
  3. Rubén JustoMay 21, 2024
  4. Re* [PATCH] add-patch: response to unknown commandJunio C Hamano, May 21, 2024
  5. Taylor BlauMay 21, 2024
  6. Junio C HamanoMay 21, 2024
  7. add-patch: enforce only one-letter response to promptsJunio C Hamano, May 21, 2024
  8. Eric SunshineMay 21, 2024
  9. Junio C HamanoMay 22, 2024
  10. Dragan SimicMay 22, 2024
  11. Junio C HamanoMay 22, 2024
  12. Dragan SimicMay 22, 2024
  13. Junio C HamanoMay 22, 2024
  14. Patrick SteinhardtMay 22, 2024
  15. Junio C HamanoMay 22, 2024
  16. add-patch: enforce only one-letter response to promptsJunio C Hamano, May 22, 2024
  17. Rubén JustoMay 22, 2024
  18. Junio C HamanoMay 22, 2024
  19. add-patch: enforce only one-letter response to promptsJunio C Hamano, May 22, 2024
  20. Patrick SteinhardtMay 23, 2024
  21. Junio C HamanoMay 23, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.