Re: [PATCH] help.c: help.autocorrect=prompt waits for user action

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 12, 2021, 19:37 UTC
Message-ID: <xmqqh7fu314a.fsf@gitster.g>
In-Reply-To: <pull.1012.git.1628640954160.gitgitgadget@gmail.com>

Show 7 quoted lines

> @@ -618,7 +622,18 @@ const char *help_unknown_cmd(const char *cmd)
>  				   _("Continuing under the assumption that "
>  				     "you meant '%s'."),
>  				   assumed);
> -		else {
> +		else if (autocorrect == AUTOCORRECT_PROMPT) {
> +			if (!isatty(STDIN_FILENO) | !isatty(STDERR_FILENO))

Don't do bitwise or when you only care about boolean outcome.

> +				exit(1);

This is not a new issue, but I think it is probably a bug in the original design of the help-unknown that we do not do this check much early in help_unknown_cmd(), to disable auto-correction in a non-interactive session. If we did so, we do not have to have this check here.

If we cannot yet come to consensus that disabling autocorrection when running non-interactively is a good idea, at least we should be able to do so only for _PROMPT case, like the attached patch at the end.

> +			char *answer;

This is decl-after-statement, but as I am suggesting to get rid of the tty check done inside this block, it will become OK.

> +			fprintf_ln(stderr, _("Assuming you meant: '%s'."),
> +				   assumed);
> +			answer = git_prompt(_("Continue? (y/N)"), PROMPT_ECHO);

Hmph, the above does not look WRONG per-se, but I wonder if it is easier for the users to see a single line, e.g.

	struct strbuf msg = STRBUF_INIT;
	strbuf_addf(&msg, _("Run '%s' instead? (y/N)"), assumed);
        answer = git_prompt(msg.buf, PROMPT_ECHO);
        strbuf_release(&msg);

Thanks.

diff --git i/help.c w/help.c
index e22ba1d246..3629cb8796 100644
--- i/help.c
+++ w/help.c
@@ -540,6 +540,13 @@ const char *help_unknown_cmd(const char *cmd)
 
 	read_early_config(git_unknown_cmd_config, NULL);
 
+	/*
+	 * Disable autocorrection prompt in a non-interactive session
+	 */
+	if ((autocorrect != AUTOCORRECT_PROMPT) &&
+	    (!isatty(0) || !isatty(2)))
+		autocorrect = AUTOCORRECT_NEVER;
+
 	if (autocorrect == AUTOCORRECT_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);