Re: What's cooking in git.git (Jan 2025, #05; Fri, 17)

From: David Aguilar <davvid@gmail.com>
Date: Jan 20, 2025, 06:53 UTC
Message-ID: <Z43y0mNhHsEdF22L@gmail.com>
In-Reply-To: <xmqqwmetgdgm.fsf@gitster.g>

On Fri, Jan 17, 2025 at 04:42:01PM -0800, Junio C Hamano wrote:

Show 10 quoted lines

> * sc/help-autocorrect-one (2025-01-13) 1 commit
>  - help: interpret boolean string values for help.autocorrect
> 
>  "[help] autocorrect = 1" used to be a way to say "please wait for
>  0.1 second after suggesting a typofix of the command name before
>  running that command"; now it means "yes, if there is a plausible
>  typofix for the command name, please run it immediately".
> 
>  Looking good except for "should 0 and false be 'tell it without doing it'?".
>  source: <pull.1869.v4.git.git.1736760824201.gitgitgadget@gmail.com>

(The text below is from the original thread; sorry I don't have it handy so I just replied here instead)

Show 8 quoted lines

> ... but would it be simpler if we made it an extended boolean, i.e.
> 
>     true, yes, on, 1  -> same as "immediate"
>     false, no, off, 0 -> same as "never"
>     immediate         -> same as what we currently do
>     never             -> same as what we currently do
>     prompt            -> same as what we currently do
>     number            -> same as what we currently do

I do think that, "0 -> same as never," makes a lot of sense from a usability perspective.

On the other hand, I don't think that, "1 -> same as immediate," is a very safe thing to do. One reason is that we should try to do the least surprising thing possible, especially when the command may be something that the user did not intend to run.

Recall that this topic was spun up because a value of "1" was interpreted as 0.1 seconds, which is effectively the same as "immediate." The original interpretation is arguably a usability issue that is not improved by these changes.

I would instead recommend that, "1 -> same as prompt," would be a safer and less surprising behavior. If the user wants "immediate" they can be explicit about it. "immediate" is the most dangerous of all of these options so adding ambiguous routes to it seems like a step backwards.

I don't really think backwards-compatibility is much of a concern here at all. It *would* be a concern if we were moving from a safe behavior to a less-safe behavior (like this patch currently does) but not so in the other direction like I'm proposing by making "1" mean "prompt".

Do we think this is a valid assessment and worth changing? If so I can try to whip up a quick patch over the existing state of "seen" to change the behavior to "prompt" when "1" is seen and "never" when "0" is seen.

-- 
David