Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

Re: [PATCH v2] branch -D: allow - as abbreviation of '@{-1}'

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:08:52

Elena Petrashen [off-list ref] writes:
quoted hunk
+static int expand_dash_shortcut(const char **argv, int dash_position)
+{
+	if (!strcmp(argv[dash_position], "-")){
+		argv[dash_position] = "@{-1}";
+		return 1;
+	}
+	return 0;
+}
 	int i;
 	int ret = 0;
+	int dash_shortcut = 0;
 	int remote_branch = 0;
 	struct strbuf bname = STRBUF_INIT;
 
@@ -213,7 +223,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 	for (i = 0; i < argc; i++, strbuf_release(&bname)) {
 		const char *target;
 		int flags = 0;
-
+		if (expand_dash_shortcut (argv, i))
+			dash_shortcut = 1;
 		strbuf_branchname(&bname, argv[i]);
I think this code special cases "-" too much.  Have you considered
doing this without "dash_shortcut" variable?  With that variable,
your code says "there is no previous" when the user says "-", but
isn't that message also appropriate when she says "@{-1}" on the
command line?  Furthermore, wouldn't the same apply to the case in
which she said "@{-4}"?

I suspect that you can check that condition immediately after
calling expand-dash-shortcut and then strbuf-branchname, in other
words, right here.  And if there is not enough branch switches, you
can say something like "you gave me @{-4} but you haven't made that
many branch switches" and continue the loop.

I _think_ strbuf_branchname() leaves "@{-<N>}" when you do not have
enough branch switches in the reflog, so perhaps

	strbuf_branchname(&bname, (!strcmp(argv[i], "-") ? "@{-1}" : argv[i]));
	if (starts_with(bname.buf, "@{-")) {
		... say "you do not have enough branch switches" here.
		... when adjusting the message to end-user input,
		... you can look at argv[i] to notice that the original
                ... input was "-".
		error(...);
		continue;
	}

or something?

That way, there is no change necessary below this line, i.e. from
here...
quoted hunk
 		if (kinds == FILTER_REFS_BRANCHES && !strcmp(head, bname.buf)) {
 			error(_("Cannot delete the branch '%s' "
@@ -231,9 +242,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 					    | RESOLVE_REF_ALLOW_BAD_NAME,
 					    sha1, &flags);
 		if (!target) {
-			error(remote_branch
-			      ? _("remote-tracking branch '%s' not found.")
-			      : _("branch '%s' not found."), bname.buf);
+			error(dash_shortcut
+				? _("There is no previous branch that could be"
+					" referred to at the moment.")
+				: remote_branch
+					? _("remote-tracking branch '%s' not found.")
+					: _("branch '%s' not found."), bname.buf);
 			ret = 1;
 			continue;
 		}
@@ -262,6 +276,10 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			       (flags & REF_ISBROKEN) ? "broken"
 			       : (flags & REF_ISSYMREF) ? target
 			       : find_unique_abbrev(sha1, DEFAULT_ABBREV));
... to here.

+			if (dash_shortcut == 1)
+			       printf( _("\nIf that happened by mistake, you may want to restore"
+				" it with:\n\ngit branch %s %s\n"), bname.buf,
+				find_unique_abbrev(sha1, DEFAULT_ABBREV));
This change can be justified only if we believe that people who say

    $ git branch -D -

by mistake are much less clueful than those who say

    $ git branch -D @{-1}
    $ git branch -D a-misspelled-branch-name

by mistake and need extra help recovering.  Is there an evidence to
support such an assumption?

I would actually understand it if this were more like

	if (advice_mistaken_branch_deletion)
        	printf(_("If you deleted the branch by mistake, you can..."));

so that everybody who ran "git branch -D" on a (wrong) branch by
mistake can get the extra help.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help