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

From: Jeff King <peff@peff.net>
Date: Aug 14, 2009, 06:55 UTC
Message-ID: <20090814065543.GA7113@coredump.intra.peff.net>
In-Reply-To: <fabb9a1e0908132340n10da3e38kfab07ab2cff18c82@mail.gmail.com>

On Thu, Aug 13, 2009 at 11:40:44PM -0700, Sverre Rabbelier wrote:

> > On the other hand, "--delete <ref>" introduces its own syntactic
> > problems.  [...]
> 
> It does indeed, and I don't think that's the way to go.

Hmm. Actually, looking at the code, we _already_ have a funny two-element syntax:

  git push origin tag v1.6.1

which, AFAICT, is totally useless, as you can just push v1.6.1 directly. I assume it's historical. I still think it's probably not a good idea to introduce a similar "delete foo".

> > Perhaps saying that "--delete=<ref>" is equivalent to ":<ref>" would be
> > a reasonable way of adding just the syntactic sugar. [...]
> 
> That would work too I guess, although it would be technically more difficult.

Really? I was thinking something as simple as:

diff --git a/builtin-push.c b/builtin-push.c
index 67f6d96..aa3784c 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -44,6 +44,12 @@ static void set_refspecs(const char **refs, int nr)
 			strcat(tag, refs[i]);
 			ref = tag;
 		}
+		if (!prefixcmp("--delete=", ref)) {
+			struct strbuf deleted = STRBUF_INIT;
+			strbuf_addstr(&deleted, ":");
+			strbuf_addstr(&deleted, skip_prefix(ref, "--delete="));
+			ref = strbuf_detach(&deleted, NULL);
+		}
 		add_refspec(ref);
 	}
 }

which is even shorter than your patch, not needing a separate option
parser.

That being said, currently parseopt will complain about that, even after
the "remote" field; we would need to pass it
PARSE_OPT_STOP_AT_NON_OPTION.

> I don't think it's that confusing either, but it's hard to stumble
> upon, yes? When you're looking at the man page for git push it is
> easier to deduct that '--delete' is what you need, than ':master'.

I think you mean "deduce", but yes, I think we have seen people complain
about the syntax in the past. I'm not against fixing it; I just want to
make sure what we introduce doesn't make any new confusion.

-Peff