On Tue, Nov 08, 2022 at 11:03:51AM +0100, Patrick Steinhardt wrote:
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -195,6 +195,13 @@ respectively, and they must begin with `refs/` when applied to `--glob`
> or `--all`. If a trailing '/{asterisk}' is intended, it must be given
> explicitly.
>
> +--exclude-hidden=[receive|uploadpack]::
> + Do not include refs that have been hidden via either one of
> + `receive.hideRefs` or `uploadpack.hideRefs` (see linkgit:git-config[1])
> + that the next `--all`, `--branches`, `--tags`, `--remotes` or `--glob`
> + would otherwise consider. This option is cleared when seeing one of
> + these pseudo-refs.
OK, so this one drops "transfer", which I think is good. But again, I
think the explanation here is subtly misleading, because it's not just
"hidden via those sections", but "hidden as receive-pack or upload-pack
would, respecting both those sections _and_ transfer".
I also wondered if we could simplify the explanation a bit by tying it
into the existing --exclude, and then we don't need that mouthful of
pseudo-ref options. So together something like:
Do not include refs that would be hidden by `receive-pack` or
`upload-pack`, by consulting the appropriate `receive.hideRefs` or
`uploadpack.hideRefs`, along with `transfer.hideRefs`. Like
`--exclude`, this option affects the next pseudo-ref option (`--all`,
`--glob`, etc), and is cleared after processing them.
But then I read the next section in the --exclude docs, which says:
The patterns given should not begin with refs/heads, refs/tags, or
refs/remotes when applied to --branches, --tags, or --remotes,
respectively, and they must begin with refs/ when applied to --glob or
--all. If a trailing /* is intended, it must be given explicitly.
Yikes. So --all is going to process "refs/heads/foo", but --branches
will see just "foo". And that means that:
git rev-list --exclude-hidden=receive --branches
will not work! Because receive.hideRefs is naming fully qualified refs,
but we'll pass unqualified ones to ref_is_hidden(). Even though that
function takes a "full" refname argument, I think that has to do with
namespaces.
I'm sure this _could_ be made to work, but I wonder if it is worth the
trouble. If it's not going to work, though, I think we'd want to detect
the situation and complain, at least for now. And likewise the
documentation needs to make clear it only works with --all and --glob.
Sorry to have misled in my initial suggestion to turn --visible-refs
into --exclude-hidden. However, I do still stand by that suggestion.
Even if we don't make it work with "--branches" now, the user-visible
framework is still there, so it becomes a matter of extending the
implementation later, rather than re-designing the options.
-Peff
