Re: [RFC PATCH 1/2] add a --delete option to git push

From: Jeff King <peff@peff.net>
Date: Aug 14, 2009, 06:33 UTC
Message-ID: <20090814063359.GA6898@coredump.intra.peff.net>
In-Reply-To: <fabb9a1e0908132324td6869aydc752f67b95546f1@mail.gmail.com>

On Thu, Aug 13, 2009 at 11:24:05PM -0700, Sverre Rabbelier wrote:

Show 10 quoted lines

> > It seems like it would be simple enough to just check whether the
> > refspec contains a colon; if so, silently leave it alone. That could
> > also protect configured refspecs, as mentioned above, but I wouldn't
> > rule out somebody have a single-name refspec in their config (in fact, I
> > think "remote.$X.push = HEAD" is reasonable -- should that delete the
> > HEAD on "git push --delete"?).
> 
> I don't think we should touch any configured refspecs, think about how
> often one would use that vs. the inconvenience of doing so
> unintentionally.

I think you are right. My previous message was sort of thinking out loud, but I think on the whole, the annoyance caused by accidental deletion is not worth it. :)

Show 9 quoted lines

> > So maybe it would make more sense for it to be "--delete <ref>" and
> > impact only a single ref. The simple case of "git push --delete foo"
> > would remain unchanged.
> 
> I thought about that, but I decided that it was both intuitive and
> convenient to be able to delete multiple refs this way.
> [...]
> I do, it's easy to make mistakes when it's more powerful, and I think
> less intuitive. I think we want this to be as intuitive as possible.

I guess I find what you are doing _more_ complex, because you are really introducing a whole new mode to push, which is "I am deleting some stuff". As opposed to some syntactic sugar to replace the confusing ":ref" syntax, which is what I thought the goal was.

On the other hand, "--delete <ref>" introduces its own syntactic problems. Is it an option, in which case you end up doing:

  git push --delete master origin

which is a bit backwards from the usual syntax. Or is it part of the refspec list, in which case:

  1. We have just disallowed a refspec called "--delete" (though to be
     fair, you have to be a little insane to use that anyway, and you
     can always call it refs/heads/--delete)).

  2. Now we don't simply have a list, one refspec per element, which
     makes things syntactically a little more complex.

Perhaps saying that "--delete=<ref>" is equivalent to ":<ref>" would be a reasonable way of adding just the syntactic sugar. I.e.:

  git push origin --delete=master

Of course, maybe the goal of a "delete mode" is useful to people. I can't think of a time when I would have used it, but then I also tend to think ":<ref>" is elegant and obvious. ;)

I dunno. I don't feel too strongly about it; mainly I was just surprised because I would have done it the other way. :)

-Peff