On Sun, Sep 14, 2025, at 00:06, Jeff King wrote: > On Sat, Sep 13, 2025 at 04:10:57PM +0200, Kristoffer Haugsbakk wrote: > >> The code in the `while(1)` speculatively adds commands to the list >> before finding out if it’s an alias. Let’s instead move it inside >> `handle_alias(...)` and in turn only run this logic when we have found >> an alias. >> >> This is not a refactor since the error output is changed; we will >> now print >> >> '<cmd1>' is aliased to '<cmd2>' >> >> while iterating in addition to the final `fatal` message. > > If you want to get rid of that last paragraph, I think it really is as > simple as checking the expanded alias new_argv[0] as soon as we see it, > like: I tried using both of these changes (the patches) but the `alias...` test suite started failing. >[snip] > You could _also_ ditch the "recursive alias" check above at that point, > though I think it produces a slightly nicer message: > > $ ./git -c alias.foo=foo foo > fatal: recursive alias: foo > > vs: > > $ ./git -c alias.foo=foo foo > fatal: alias loop detected: expansion of 'foo' does not terminate: > foo <== I didn’t try these. >[snip] > I dunno. Maybe it is not worth tinkering with too much. But the "this > changes the output" justification from your proposed message seems to me > an indication that the refactor is a little iffy. I ended up with not changing it for v5. I missed the first time around that this informational message is only “logged” in the specific case of `<git cmd> -h`. In turn you only get one more line of output when you are (1) chaining deprecated aliases, and (2) making a loop. v5 still mentions this but I try to put it off to the side, in other words a footnote. Thanks
