From: Jeff King <peff@peff.net>
Date: Mon, 08 Sep 2025 21:11:41 GMT
Subject: Re: [PATCH v3 3/8] git: allow alias-shadowing deprecated builtins
Message-ID: <20250908211141.GB1327610@coredump.intra.peff.net>
In-Reply-To: <xmqqcy80wu0y.fsf@gitster.g>

On Mon, Sep 08, 2025 at 01:47:57PM -0700, Junio C Hamano wrote:

> kristofferhaugsbakk@fastmail.com writes:
> 
> > +static int is_deprecated_command(const char *cmd)
> > +{
> > +	return !strcmp(cmd, "whatchanged") ||
> > +	       !strcmp(cmd, "pack-redundant");
> > +}
> 
> This is somewhat a shame as you introduced the DEPRECATED bit to
> annotate entries in "struct cmd_struct commands[]" array.  Shouldn't
> this be moved to git.c and taught to consult that array instead?

Hmm, I did not see the original patch, as it does not seem to have hit
the list (or maybe vger is just slow today).

But yeah, if we have a bit in the cmd_struct array, that might be a
nicer spot to match. It may or may not be trivial. When I posted my
"something like this" patch I first tried to do it that way, but there
are a lot of ordering constraints and the parts where we resolved the
cmd_struct were not the right spots. OTOH, it would probably not be that
hard to just do:

  static int is_deprecated_command(const char *cmd)
  {
	struct cmd_struct *builtin = get_builtin(cmd);
	return builtin && (builtin->flags & DEPRECATED);

  }

In theory we could also have a deprecated non-builtin (i.e., implemented
as an external program). And then we really would have to match by name.
But I kind of doubt that will happen in practice (and it would still be
possible to add a string match later if it did).

-Peff