From: Jeff King <peff@peff.net>
Date: Fri, 14 Aug 2009 06:33:59 GMT
Subject: Re: [RFC PATCH 1/2] add a --delete option to git push
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:

> > 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. :)

> > 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