Re: [PATCH] fetch: add new config option fetch.all

From: Tamino Bauknecht <dev@tb6.eu>
Date: Jan 4, 2024, 20:18 UTC
Message-ID: <70f17f0c-2226-41eb-ae08-348c794a3411@tb6.eu>
In-Reply-To: <xmqqv8896jqo.fsf@gitster.g>

On 1/4/24 19:23, Junio C Hamano wrote:> This sounds like a usability annoyance for forks that use --all some

Show 8 quoted lines

> of the time and not always, as they now have to remember not just to
> pass something to countermand the configured fetch.all but what that
> something exactly is (i.e., the name of the remote they fetch from
> by default), which makes fetch.all less appealing.  I wonder if we
> can instead take "--no-all" from the command line to make configured
> fetch.all ignored (hence, giving an explicit remote will fetch from
> there, and not giving any remote will fetch from whereever the
> command will fetch from with "git -c fetch.all=false fetch")?

I don't think I fully understand the scenario you describe here. But I see that the change would disallow users to fetch only the default remote without its name in a straight-forward way - either your proposed solution to overrule the config value or using something like `git config "branch.$(git branch --show-current).remote"` in combination with `git fetch` would be workarounds. Do you think it is worth adding a flag for it? I can't really think of a real-world use case for it. E.g. the config option "fetch.prune" also doesn't have anything to counteract it (as far as I see).

If a flag is necessary, I think something like "--default-only" (or similar) might be more descriptive than "--no-all".

> Missing sign-off.

Sorry, thanks for pointing it out.

> And we should say that this configuration variable defaults to false.

Will do so.

Show 5 quoted lines

> This conditional cascade will probably need to change when we allow
> "--no-all" to countermand the configured fetch.all anyway, so I
> won't worry about it now, but it looks somewhat convoluted that we
> have to re-check "all" many times, which was the point of the
> original that implemented this as a nested conditional.

It's probably because of the tab width of 8 that I feel like three indentation levels are already too much. I'll use Taylor's suggestion to keep the `argc` check as-is (although two checks will still be necessary). As an alternative I thought about modifying the current behavior of "--all" in combination with an explicit remote as well, but discarded that idea because it might be less intuitive than the error.