Re: [PATCH v2 05/12] builtin/show-ref: refactor `--exclude-existing` options

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]

On Mon, Oct 30, 2023 at 02:55:21PM -0400, Taylor Blau wrote:
> On Thu, Oct 26, 2023 at 11:56:37AM +0200, Patrick Steinhardt wrote:
> > @@ -95,6 +94,11 @@ static int add_existing(const char *refname,
> >  	return 0;
> >  }
> >
> > +struct exclude_existing_options {
> > +	int enabled;
> > +	const char *pattern;
> > +};
> > +
> 
> Thinking on my earlier suggestion more, I wondered if using the
> OPT_SUBCOMMAND() function might make things easier to organize and
> eliminate the need for things like .enabled or having to define structs
> for each of the sub-commands.
> 
> But I don't think that this is (easily) possible to do, since
> `--exclude-existing` is behind a command-line option, not a separate
> mode (e.g. "commit-graph verify", not "commit-graph --verify"). So I
> think you *could* make it work with some combination of OPT_SUBCOMMAND
> and callbacks to set the function pointer yourself when given the
> `--exclude-existing` option. But I think that's sufficiently gross as to
> not be worth it.

Yeah, agreed. Honestly, while working on this series I had the dream of
just discarding git-show-ref(1) in favor of a new command with proper
subcommands as we tend to use them nowadays:

    - `git references list <patterns>...` replaces `git show-ref
      <pattern>`.

    - `git references filter <pattern>` replaces `git show-ref
      --exclude-existing` and filters references from stdin.

    - `git references exists <ref>` checks whether a reference exists or
      not and replaces `git show-ref --exists`.

This would make for a much more enjoyable UX. It'd also be a more
natural home for potential future additions:

    - `git references show <ref>` allows you to show the contents of the
      reference without resolving it, regardless of whether it's a
      direct or a symbolic reference.

    - `git references count <patterns>...` allows you to count refs
      patching the pattern.

I shied away though because it would be a much more controversial topic
that would potentially result in lots of bikeshedding. Now if everyone
was enthusiastic about this idea I'd still be happy to do it, even
though it derails the topic even further from its original intent to
just fix a bunch of tests. But unless that happens, I'll continue to
stick with the mediocre UI we have in git-show-ref(1).

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]