Thread (26 messages) flat view 26 messages, 4 authors, 2018-04-04

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

From: Eric Sunshine <hidden>
Date: 2018-03-25 01:35:05

On Sat, Mar 24, 2018 at 2:38 PM, Kaartic Sivaraam
[off-list ref] wrote:
When rebasing interacitvely (rebase -i), "git branch -l" prints a line
The "git branch -l" threw me since "-l" is short for --create-reflog.
I'm guessing you meant "git branch --list".
indicating the current branch being rebased. This works well when the
interactive rebase was intiated when a local branch is checked out.

This doesn't play well when the rebase was initiated on a remote
branch or an arbitrary commit that is not pointed to by a local
branch.
A shorter way of saying "arbitrary commit ... not pointed at by local
branch" would be "detached HEAD".
In this case "git branch -l" tries to print the name of a
branch using an unintialized variable and thus tries to print a "null
pointer string". As a consequence, it does not provide useful
information while also inducing undefined behaviour.

So, print the commit from which the rebase started when interactive
rebasing a non-local branch.
Makes sense. The commit message gives enough information for the
reader to understand the problem easily.
quoted hunk ↗ jump to hunk
Signed-off-by: Kaartic Sivaraam <redacted>
---
diff --git a/ref-filter.c b/ref-filter.c
@@ -1310,8 +1310,16 @@ char *get_head_description(void)
        wt_status_get_state(&state, 1);
        if (state.rebase_in_progress ||
            state.rebase_interactive_in_progress)
+       {
Style: attach '{' to the line above it (don't make it standalone)
+               const char *rebasing = NULL;
+               if (state.branch != NULL)
+                       rebasing = state.branch;
+               else
+                       rebasing = state.detached_from;
+
                strbuf_addf(&desc, _("(no branch, rebasing %s)"),
-                           state.branch);
+                           rebasing);
You could collapse the whole thing back down to:

    strbuf_addf(&desc, _("(no branch, rebasing %s)"),
        state.branch ? state.branch : state.detached_from);

which means you don't need the 'rebasing' variable or the braces.
+       }
        else if (state.bisect_in_progress)
Style: cuddle 'else' with '}': } else
                strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
                            state.branch);
Can we have a couple new tests: one checking "git branch --list" for
the typical case (when rebasing off a named branch) and one checking
when rebasing from a detached HEAD?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help