Re: [PATCH v2] help: include arguments in autocorrect=prompt message

[Junio C Hamano <gitster@pobox.com>]

Jishnu C K <jishnuck26@gmail.com> writes:

> v2: Reworked as an incremental improvement to the existing
> autocorrect=prompt code path rather than a parallel reimplementation,
> per feedback from Junio and Justin.
>
> ---
> From a4e8fb6fd6dd6a501e565c7500cbf927d7cb0b42 Mon Sep 17 00:00:00 2001
> From: calicomills <jishnuck26@gmail.com>
> Date: Fri, 19 Jun 2026 13:01:40 +0530
> Subject: [PATCH v2 v2] help: include arguments in autocorrect=prompt message

To learn what a typical v2 of a single-patch topic should look like,
see

  https://lore.kernel.org/git/aipTOsH8LKTSwglj@collabora.com/

for an example.

 - Having auxiliary comments explaining why there is this v2,
   including description of the difference since v1, is good, but
   have it below the three-dash line after your sign off, not at the
   beginning.

 - Please do not include "From a4e8fb6f..." line, which is meant as
   a separator in multi-patch output from the git format-patch
   command; knowing the exact commit object name you took the patch
   from in your repository would not help anybody.

 - Do not include "From:" in the body of the message either, unless
   you are relaying somebody else's patch, i.e., when the From
   e-mail header (you) does not name the person who wrote the patch
   (somebody else).

 - Do not include "Date:" in the body of the message either, as that
   is the timestamp you wrote the change, but we care more about the
   time when the general public first saw the patch, which is in the
   e-mail header already.

 - Do not include "Subject:" in the body of the message either, as
   that should be on the Subject e-mail header.

> When 'help.autocorrect=prompt' is configured and the user mistypes
> a git command, the prompt currently shows only the corrected command
> name:
>
>   Run 'checkout' instead [y/N]?
>
> This leaves the user unsure whether their original arguments will be
> preserved. Update the prompt to include the full corrected invocation:
>
>   Run 'git checkout neo' instead [y/N]?
>
> The help_unknown_cmd() signature is updated to accept the args vector
> so the prompt can show the original arguments alongside the corrected
> command name. Callers that do not have access to the args (e.g.
> builtin/help.c) pass NULL, which is handled gracefully.
>
> Signed-off-by: calicomills <jishnuck26@gmail.com>

Documentation/SubmittingPatches[[real-name]] prefers to see us
interacting with humans with real-sounding names, not handles.

> ---
>  help.c                      | 49 +++++++++++++----------------------
>  t/t9003-help-autocorrect.sh | 51 +++++--------------------------------
>  2 files changed, 23 insertions(+), 77 deletions(-)
>
> diff --git a/help.c b/help.c
> index 30f32a7206..9ea4c076e1 100644
> --- a/help.c
> +++ b/help.c
> @@ -739,7 +739,16 @@ char *help_unknown_cmd(const char *cmd, const struct strvec *args)
>  		else if (cfg.autocorrect == AUTOCORRECT_PROMPT) {
>  			char *answer;
>  			struct strbuf msg = STRBUF_INIT;
> -			strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
> +			struct strbuf full_cmd = STRBUF_INIT;
> +			strbuf_addstr(&full_cmd, assumed);
> +			if (args) {
> +				for (size_t j = 1; j < args->nr; j++) {
> +					strbuf_addch(&full_cmd, ' ');
> +					strbuf_addstr(&full_cmd, args->v[j]);
> +				}
> +			}
> +			strbuf_addf(&msg, _("Run 'git %s' instead [y/N]? "), full_cmd.buf);
> +			strbuf_release(&full_cmd);

This change seems to match what is explained in the proposed log
message.  Instead of giving the "assumed" command without its
arguments, the full command line is gprepared to be given in 'msg'.

But if we really wanted to let these be cut-and-paste, don't you
need to shell-quote the command line?  If the user typed

	$ git comit -m "Hello world"

the above makes

	Run 'git commit -m hello world' instead [y/N]?

which would record the change made only to the file "world" with log
message "hello", which is not what the user wanted to do.

> @@ -762,37 +771,13 @@ char *help_unknown_cmd(const char *cmd, const struct strvec *args)
>  	fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
>  
>  	if (SIMILAR_ENOUGH(best_similarity)) {
> -		if (n == 1 && isatty(0) && isatty(2)) {
> -			char *answer;
> -			struct strbuf msg = STRBUF_INIT;
> -			struct strbuf full_cmd = STRBUF_INIT;
> -			strbuf_addstr(&full_cmd, main_cmds.names[0]->name);
> -			if (args) {
> -				for (size_t j = 1; j < args->nr; j++) {
> -					strbuf_addch(&full_cmd, ' ');
> -					strbuf_addstr(&full_cmd, args->v[j]);
> -				}
> -			}
> -			strbuf_addf(&msg, _("\nDid you mean 'git %s'? [y/N] "),
> -				    full_cmd.buf);
> -			strbuf_release(&full_cmd);
> -			answer = git_prompt(msg.buf, PROMPT_ECHO);
> -			strbuf_release(&msg);
> -			if (starts_with(answer, "y") || starts_with(answer, "Y")) {
> -				char *assumed = xstrdup(main_cmds.names[0]->name);
> -				cmdnames_release(&cfg.aliases);
> -				cmdnames_release(&main_cmds);
> -				cmdnames_release(&other_cmds);
> -				return assumed;
> -			}

What is this removal about?  The original used to give interactive
prompt to let the user say "Yes", but with this part removed, it no
longer offers the "well we have only one candidate, do you want to
run that one?", and you instead ...

> -		} else {
> -			fprintf_ln(stderr,
> -				   Q_("\nThe most similar command is",
> -				      "\nThe most similar commands are",
> -				   n));
> -			for (i = 0; i < n; i++)
> -				fprintf(stderr, "\t%s\n", main_cmds.names[i]->name);
> -		}

... only give "these are the possible candidates?"

> +		fprintf_ln(stderr,
> +			   Q_("\nThe most similar command is",
> +			      "\nThe most similar commands are",
> +			   n));
> +
> +		for (i = 0; i < n; i++)
> +			fprintf(stderr, "\t%s\n", main_cmds.names[i]->name);
>  	}

Puzzled.

Worse, this [v2] does not even apply cleanly to any of the trees we
have.

Aha!  Is the removal I see above a mere "oops, this was wrong, so
remove it" done on top of a previous iteration of the patch?

Please do not do that.  It would mean we will keep unwanted code
that went into a wrong direction (which is v1) in our history.

The development may wander around in different directions like a
drunken man, changing course every time patches are updated, until
it finally gets right, but the name of the game around here, before
your change is merged to 'next', is to "pretend to be a more perfect
developer than you actually are" ;-).  You can pretend that you
never made a design mistake you made in [v1], and instead directly
arrived at a good state from the state of our public tree.  That
means [v2] (and any subsequent rerolls, until your seires gets
merged to 'next') must apply directly without any of the older
iterations to 'master' (or if it is a bugfix, 'maint') branch.

Thanks.