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

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

quoted

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

quoted

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