Thread (66 messages) 66 messages, 3 authors, 2026-04-23

Re: [PATCH v3 3/8] help: move tty check for autocorrection to autocorrect.c

From: Karthik Nayak <hidden>
Date: 2026-03-10 14:06:20

Jiamu Sun [off-list ref] writes:
quoted hunk ↗ jump to hunk
TTY checking is the autocorrect config parser's responsibility. It must
ensure the parsed value is correct and reliable. Thus, move the check to
autocorr_resolve_config().

Signed-off-by: Jiamu Sun <redacted>
---
In parse_autocorrect() I kept the if/else if cascade. In my opinion,
this is not a style change. It's a control-flow clarification, because
those returns describe different states of the mode. Chaining them
together is better than leaving them discrete.

Also, I dropped the float cast in the delay calculation, which is
redundant.

 autocorrect.c | 50 +++++++++++++++++++++++++++++---------------------
 autocorrect.h | 20 ++++++++++++++------
 help.c        | 17 ++++++-----------
 3 files changed, 49 insertions(+), 38 deletions(-)
diff --git a/autocorrect.c b/autocorrect.c
index 1037f032019b..50d7f116d85d 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -6,7 +6,7 @@
 #include "prompt.h"
 #include "gettext.h"

-static int parse_autocorrect(const char *value)
+static enum autocorr_mode parse_autocorrect(const char *value)
 {
 	switch (git_parse_maybe_bool_text(value)) {
 		case 1:
@@ -19,41 +19,49 @@ static int parse_autocorrect(const char *value)

 	if (!strcmp(value, "prompt"))
 		return AUTOCORRECT_PROMPT;
-	if (!strcmp(value, "never"))
+	else if (!strcmp(value, "never"))
 		return AUTOCORRECT_NEVER;
-	if (!strcmp(value, "immediate"))
+	else if (!strcmp(value, "immediate"))
 		return AUTOCORRECT_IMMEDIATELY;
-	if (!strcmp(value, "show"))
+	else if (!strcmp(value, "show"))
 		return AUTOCORRECT_SHOW;
-
-	return 0;
+	else
+		return AUTOCORRECT_DELAY;
 }
Okay so since we introduce an enum we use that here.
quoted hunk ↗ jump to hunk
 void autocorr_resolve_config(const char *var, const char *value,
 			     const struct config_context *ctx, void *data)
 {
-	int *out = data;
+	struct autocorr *conf = data;
+
+	if (strcmp(var, "help.autocorrect"))
+		return;
+
+	conf->mode = parse_autocorrect(value);

-	if (!strcmp(var, "help.autocorrect")) {
-		int v = parse_autocorrect(value);
+	/*
+	 * Disable autocorrection prompt in a non-interactive session.
+	 */
+	if (conf->mode == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2)))
+		conf->mode = AUTOCORRECT_NEVER;

-		if (!v) {
-			v = git_config_int(var, value, ctx->kvi);
-			if (v < 0 || v == 1)
-				v = AUTOCORRECT_IMMEDIATELY;
-		}
+	if (conf->mode == AUTOCORRECT_DELAY) {
+		conf->delay = git_config_int(var, value, ctx->kvi);

-		*out = v;
+		if (!conf->delay)
+			conf->mode = AUTOCORRECT_SHOW;
+		else if (conf->delay <= 1)
+			conf->mode = AUTOCORRECT_IMMEDIATELY;
 	}
 }

-void autocorr_confirm(int autocorrect, const char *assumed)
+void autocorr_confirm(struct autocorr *conf, const char *assumed)
 {
-	if (autocorrect == AUTOCORRECT_IMMEDIATELY) {
+	if (conf->mode == AUTOCORRECT_IMMEDIATELY) {
 		fprintf_ln(stderr,
 			   _("Continuing under the assumption that you meant '%s'."),
 			   assumed);
-	} else if (autocorrect == AUTOCORRECT_PROMPT) {
+	} else if (conf->mode == AUTOCORRECT_PROMPT) {
 		char *answer;
 		struct strbuf msg = STRBUF_INIT;
@@ -63,10 +71,10 @@ void autocorr_confirm(int autocorrect, const char *assumed)

 		if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
 			exit(1);
-	} else {
+	} else if (conf->mode == AUTOCORRECT_DELAY) {
 		fprintf_ln(stderr,
 			   _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
-			   (float)autocorrect / 10.0, assumed);
-		sleep_millisec(autocorrect * 100);
+			   conf->delay / 10.0, assumed);
+		sleep_millisec(conf->delay * 100);
 	}
 }
diff --git a/autocorrect.h b/autocorrect.h
index 45609990c77e..ce4a68379f2f 100644
--- a/autocorrect.h
+++ b/autocorrect.h
@@ -1,16 +1,24 @@
 #ifndef AUTOCORRECT_H
 #define AUTOCORRECT_H

-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
-#define AUTOCORRECT_IMMEDIATELY (-1)
-
 struct config_context;

+enum autocorr_mode {
+	AUTOCORRECT_SHOW,
+	AUTOCORRECT_NEVER,
+	AUTOCORRECT_PROMPT,
+	AUTOCORRECT_IMMEDIATELY,
+	AUTOCORRECT_DELAY,
+};
+
+struct autocorr {
+	enum autocorr_mode mode;
+	int delay;
+};
+
I would say the naming doesn't indicate what it is used for. How about
'autocorrect_config'?

[snip]

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help