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

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 31, 2023, 08:10 UTC
Message-ID: <ZUC2ePXa4t8OElhu@tanuki>
In-Reply-To: <ZT/8GbzAmBq0aRIK@nand.local>

On Mon, Oct 30, 2023 at 02:55:21PM -0400, Taylor Blau wrote:

Show 23 quoted lines

> 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