On 7/26/2022 10:44 AM, Ævar Arnfjörð Bjarmason wrote:
>
> On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:
>
>> This was previously reduced by adding the log.excludeDecoration config
>> option and modifying that config in git maintenance's prefetch task (to hide
>> refs/prefetch/*). I then followed that pattern again for the bundle URI
>> feature [1], but this caught some reviewers by surprise as an unfortunate
>> side-effect. This series is a way to roll back the previous decision to use
>> log.excludeDecoration and instead use tighter filters by default.
>>
>> As noted in the last patch, the current design ignores the new filters if
>> there are any previously-specified filters. This includes the
>> log.excludeDecorations=refs/prefetch/ set by the git maintenance command.
>> This means that users who ran that command in their repo will not get the
>> benefits of the more strict filters. While we stop writing
>> log.excludeDecorations, we don't remove existing instances of it.
>
> Leaving aside the question of these magic refs, and if we need new ones
> (e.g. refs/bundle/*) I have sometimes made use of out-of-standard
> refspace refs.
>
> E.g. when I build git I create refs/built-tags/* tag object refs
> (i.e. not in refs/tags/*), which is a neat way to get "git tag -l" and
> the like to ignore it.
>
> But to still have it show decorated in logs (e.g. I'll see what my
> "private" branch is at), and "for-each-ref --contains" still knows about
> it.
You also have the unfortunate UX of having the refs spelled out entirely
("refs/<special-place>/..." instead of "<special-place>/..." like how
"refs/remotes/" is dropped from remote refs) and not having special color.
But that's beside the point.
> Now, that's a rather obscure use-case, and I suspect other "special
> refs" are similarly obscure (e.g. GitLab's refs/keep-around/* comes to
> mind).
>
> But I think this change is going about it the wrong way, let's have a
> list of refs that Git knows about as magical, instead of assuming that
> we can ignore everything that's not on a small list of things we're
> including.
>
> Wouldn't that give you what you want, and not exclude these sorts of
> custom refs unexpectedly for users?
Instead of keeping track of an ever-growing list of exclusions, instead
making a clear list of "this is what most users will want for their
decorations" is a better approach.
Users who know how to create custom refs outside of this space have the
capability to figure out how to show their special refs. My general ideas
for designing these kinds of features is to have a default that is focused
on the typical user while giving config options for experts to tweak those
defaults.
You're right that this series perhaps leaves something to be desired in
that second part, since there isn't an easy _config-based_ way to enable
all decorations (or a small additional subset).
>> I'm interested if anyone has another way around this issue, or if we
>> consider adding the default filter as long as no --decorate=refs options are
>> specified.
>
> I think the resulting UX here is bad, in that we ship hardcoded list of
> these if you don't specify the config in 2/3. So I can do:
>
> -c log.excludeDecoration=this-will-never-match
>
> To "clear" the list, but not this:
>
> -c log.excludeDecoration=
The thing that I forgot to do, but had considered was adding a
--decorate-all option to allow clearing the default filters from the
command line. You can already do "--decorate-refs=refs" to get everything
(except HEAD).
As far as config goes, we could also create a log.includeDecoration key,
but we'd want to consider it to populate the same part of the filtering
algorithm. Similar to having any instance of log.excludeDecoration, this
would clear the default list. To get all decorations again, you could add
this to your config file:
[log]
includeDecoration = refs
includeDecoration = HEAD
Alternatively, we could instead create a "filter default" option such as
"log.decorateFilter = (core|all)" where "core" is the default set being
considered by this series, and "all" is the empty filter.
Thanks,
-Stolee
