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
- signature.asc [application/pgp-signature] 690 bytes