[PATCH RFC v2] builtin-push: add --delete as syntactic sugar for :foo

["Jan Krüger" <jk@jk.gs>]

Refspecs without a source side have been reported as confusing by many.
As an alternative, this adds support for commands like:

    git push origin --delete somebranch

Specifically, --delete will prepend a colon to all colon-less refspecs
given on the command line, and will refuse to accept refspecs with
colons to prevent undue confusion.

Signed-off-by: Jan Krüger <jk@jk.gs>
---
All good points, and I actually remembered to catch the colon-refspec
case a few minutes after sending the patch, but didn't have time to fix
it up. (Sorry for not considering the old thread; I had gotten
unsubscribed from the list back then).

I think with this change it becomes much saner. And no, I'm not
proposing to add --rename and --copy, too. ;)

The new error message is not completely technically correct since it's
still a refspec we're taking, we just force it to be without a colon.
On the other hand, this error message will hopefully make much more
sense to people who don't know all the background, and it's not too
horribly wrong either.

(Sverre added to Cc list from subthread)

 builtin-push.c        |   16 ++++++++++++++++
 t/t5516-fetch-push.sh |    6 ++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 8631c06..e40bfe8 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -15,6 +15,7 @@ static const char * const push_usage[] = {
 };
 
 static int thin;
+static int deleterefs;
 static const char *receivepack;
 
 static const char **refspec;
@@ -44,6 +45,15 @@ static void set_refspecs(const char **refs, int nr)
 			strcat(tag, refs[i]);
 			ref = tag;
 		}
+		if (deleterefs && !strchr(ref, ':')) {
+			char *delref;
+			int len = strlen(refs[i] + 1);
+			delref = xmalloc(len);
+			strcpy(delref, ":");
+			strcat(delref, refs[i]);
+			ref = delref;
+		} else if (deleterefs)
+			die("--delete only accepts plain target ref names");
 		add_refspec(ref);
 	}
 }
@@ -181,6 +191,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT( 0 , "all", &flags, "push all refs", TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
+		OPT_BOOLEAN( 0, "delete", &deleterefs, "delete refs"),
 		OPT_BOOLEAN( 0 , "tags", &tags, "push tags (can't be used with --all or --mirror)"),
 		OPT_BIT('n' , "dry-run", &flags, "dry run", TRANSPORT_PUSH_DRY_RUN),
 		OPT_BIT( 0,  "porcelain", &flags, "machine-readable output", TRANSPORT_PUSH_PORCELAIN),
@@ -193,6 +204,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 
+	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
+		die("--delete is incompatible with --all, --mirror and --tags");
+	if (deleterefs && argc < 2)
+		die("--delete doesn't make sense without any refs");
+
 	if (tags)
 		add_refspec("refs/tags/*");
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 6889a53..aa1450a 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -546,6 +546,12 @@ test_expect_success 'allow deleting an invalid remote ref' '
 
 '
 
+test_expect_success 'allow deleting a ref using --delete' '
+	mk_test heads/master &&
+	git push testrepo --delete master &&
+	(cd testrepo && test_must_fail git rev-parse --verify refs/heads/master)
+'
+
 test_expect_success 'warn on push to HEAD of non-bare repository' '
 	mk_test heads/master
 	(cd testrepo &&
-- 
1.6.5.2.155.gbb47.dirty