| Message ID | CAC05386q2iGoiJ_fRgwoOTF23exEN2D1+oh4VjajEvYQ58O1TQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | branch: colorize branches checked out in a linked working tree the same way as the current branch is colorized | expand |
On Thu, Sep 27 2018, Nickolai Belakovski wrote: > In order to more clearly display which branches are active, the output > of git branch is modified to colorize branches checked out in any linked > worktrees with the same color as the current branch. > > This is meant to simplify workflows related to worktree, particularly > due to the limitations of not being able to check out the same branch in > two worktrees and the inability to delete a branch checked out in a > worktree. When performing branch operations like checkout and delete, it > would be useful to know more readily if the branches in which the user > is interested are already checked out in a worktree. > > The git worktree list command contains the relevant information, however > this is a much less frquently used command than git branch. > > Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com> Sounds cool, b.t.w. would be neat-o to have some screenshot uploaded to imgur or whatever just to skim what it looks like before/after. > diff --git a/builtin/branch.c b/builtin/branch.c > index 4fc55c350..65b58ff7c 100644 > --- a/builtin/branch.c > +++ b/builtin/branch.c > @@ -334,11 +334,36 @@ static char *build_format(struct ref_filter > *filter, int maxwidth, const char *r > struct strbuf local = STRBUF_INIT; > struct strbuf remote = STRBUF_INIT; > > - strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else) %s%%(end)", > - branch_get_color(BRANCH_COLOR_CURRENT), > - branch_get_color(BRANCH_COLOR_LOCAL)); > - strbuf_addf(&remote, " %s", > - branch_get_color(BRANCH_COLOR_REMOTE)); > + // Prepend the current branch of this worktree with "* " and > all other branches with " " We use /* ... */ C comments, not C++-style // (well, it's in C now, but not the ancient versions we need to support). It also seems all of this patch was copy/pasted into GMail or something, it has wrapping and doesn't apply with "git am". Also most/all of these comments I'd say we could better do without, i.e. the ones explaining basic code flow that's easy to see from the code itself.
Will do re: screenshot when I get home, although it's pretty easy to imagine, the git branch output will have one other branch colored in green, bit without the asterisk (for one linked worktree) :) Also will do re: changing comments to /**/ (didn't know // was from C++, TIL) and I'll clean up the comments to remove some of the more obvious ones, but I'll try to keep a comment explaining the basic flow of creating a nest if statement to evaluate worktree refs for color. And yes, I copy/pasted into gmail. I was having trouble setting up send-email, but I think I may have it figured out now. Should I create a new thread with send-email? Or maybe reply to this one (I can do that by specifying the Message-ID to reply to right? This is my first time using this workflow, so I appreciate your patience :) )? Thanks for the feedback! On Thu, Sep 27, 2018 at 8:33 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Thu, Sep 27 2018, Nickolai Belakovski wrote: > > > In order to more clearly display which branches are active, the output > > of git branch is modified to colorize branches checked out in any linked > > worktrees with the same color as the current branch. > > > > This is meant to simplify workflows related to worktree, particularly > > due to the limitations of not being able to check out the same branch in > > two worktrees and the inability to delete a branch checked out in a > > worktree. When performing branch operations like checkout and delete, it > > would be useful to know more readily if the branches in which the user > > is interested are already checked out in a worktree. > > > > The git worktree list command contains the relevant information, however > > this is a much less frquently used command than git branch. > > > > Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com> > > Sounds cool, b.t.w. would be neat-o to have some screenshot uploaded to > imgur or whatever just to skim what it looks like before/after. > > > diff --git a/builtin/branch.c b/builtin/branch.c > > index 4fc55c350..65b58ff7c 100644 > > --- a/builtin/branch.c > > +++ b/builtin/branch.c > > @@ -334,11 +334,36 @@ static char *build_format(struct ref_filter > > *filter, int maxwidth, const char *r > > struct strbuf local = STRBUF_INIT; > > struct strbuf remote = STRBUF_INIT; > > > > - strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else) %s%%(end)", > > - branch_get_color(BRANCH_COLOR_CURRENT), > > - branch_get_color(BRANCH_COLOR_LOCAL)); > > - strbuf_addf(&remote, " %s", > > - branch_get_color(BRANCH_COLOR_REMOTE)); > > + // Prepend the current branch of this worktree with "* " and > > all other branches with " " > > > We use /* ... */ C comments, not C++-style // (well, it's in C now, but > not the ancient versions we need to support). > > It also seems all of this patch was copy/pasted into GMail or something, > it has wrapping and doesn't apply with "git am". > > Also most/all of these comments I'd say we could better do without, > i.e. the ones explaining basic code flow that's easy to see from the > code itself.
On Thu, Sep 27, 2018 at 5:15 PM Nickolai Belakovski <nbelakovski@gmail.com> wrote: > > In order to more clearly display which branches are active, the output > of git branch is modified to colorize branches checked out in any linked > worktrees with the same color as the current branch. My first thought was "how do I know which branch I'm on then if they are all green?" but then the current worktree's branch would have a "*" in front while other worktree's do not. Perhaps worth mentioning in the commit message. It may be better though to have a different color code for other worktree's branch, we can still default the color to green, but people who rely on colors rather than "*" can choose a different color.
On Thu, Sep 27 2018, Nickolai Belakovski wrote: > Will do re: screenshot when I get home, although it's pretty easy to > imagine, the git branch output will have one other branch colored in green, > bit without the asterisk (for one linked worktree) :) > > Also will do re: changing comments to /**/ (didn't know // was from C++, > TIL) and I'll clean up the comments to remove some of the more obvious > ones, but I'll try to keep a comment explaining the basic flow of creating > a nest if statement to evaluate worktree refs for color. > > And yes, I copy/pasted into gmail. I was having trouble setting up > send-email, but I think I may have it figured out now. Should I create a > new thread with send-email? Or maybe reply to this one (I can do that by > specifying the Message-ID to reply to right? You'd run git format-patch master..your-topic with --subject-prefix="PATCH v2" and --in-reply-to="<CAC05386q2iGoiJ_fRgwoOTF23exEN2D1+oh4VjajEvYQ58O1TQ@mail.gmail.com>". Then it'll show up in reply to your v1. You can also for an easier experience do this via GitGitGadget, see https://github.com/gitgitgadget/gitgitgadget looking at its code it seems to have some way to reference a Message-ID, but I don't know how to trigger that. > This is my first time using this workflow, so I appreciate your > patience :) )? No worries, happy to help. > On Thu, Sep 27, 2018 at 8:33 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> > wrote: > >> >> On Thu, Sep 27 2018, Nickolai Belakovski wrote: >> >> > In order to more clearly display which branches are active, the output >> > of git branch is modified to colorize branches checked out in any linked >> > worktrees with the same color as the current branch. >> > >> > This is meant to simplify workflows related to worktree, particularly >> > due to the limitations of not being able to check out the same branch in >> > two worktrees and the inability to delete a branch checked out in a >> > worktree. When performing branch operations like checkout and delete, it >> > would be useful to know more readily if the branches in which the user >> > is interested are already checked out in a worktree. >> > >> > The git worktree list command contains the relevant information, however >> > this is a much less frquently used command than git branch. >> > >> > Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com> >> >> Sounds cool, b.t.w. would be neat-o to have some screenshot uploaded to >> imgur or whatever just to skim what it looks like before/after. >> >> > diff --git a/builtin/branch.c b/builtin/branch.c >> > index 4fc55c350..65b58ff7c 100644 >> > --- a/builtin/branch.c >> > +++ b/builtin/branch.c >> > @@ -334,11 +334,36 @@ static char *build_format(struct ref_filter >> > *filter, int maxwidth, const char *r >> > struct strbuf local = STRBUF_INIT; >> > struct strbuf remote = STRBUF_INIT; >> > >> > - strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else) >> %s%%(end)", >> > - branch_get_color(BRANCH_COLOR_CURRENT), >> > - branch_get_color(BRANCH_COLOR_LOCAL)); >> > - strbuf_addf(&remote, " %s", >> > - branch_get_color(BRANCH_COLOR_REMOTE)); >> > + // Prepend the current branch of this worktree with "* " and >> > all other branches with " " >> >> >> We use /* ... */ C comments, not C++-style // (well, it's in C now, but >> not the ancient versions we need to support). >> >> It also seems all of this patch was copy/pasted into GMail or something, >> it has wrapping and doesn't apply with "git am". >> >> Also most/all of these comments I'd say we could better do without, >> i.e. the ones explaining basic code flow that's easy to see from the >> code itself. >>
On Thu, Sep 27, 2018 at 08:13:13AM -0700, Nickolai Belakovski wrote: > In order to more clearly display which branches are active, the output > of git branch is modified to colorize branches checked out in any linked > worktrees with the same color as the current branch. I think the goal makes sense. Do we want to limit this to git-branch, though? Ideally any output you get from git-branch could be replicated with for-each-ref (or with a custom "branch --format"). I.e., could we have a format in ref-filter that matches HEAD, but returns a distinct symbol for a worktree HEAD? That would allow a few things: - custom --formats for for-each-ref and branch could reuse the logic - we could show the symbol (in place of "*") even when color is not enabled - it should be much faster if there are a lot of worktrees; your patch does a linear if/else chain to look at each worktree, and it does it in the format-language, which is much slower than actual C. :) Something like the patch below. I just picked "+" arbitrarily, but any character would do (I avoided "*" just to make it visually distinct from the current-worktree HEAD). I've left plugging this into git-branch's default format as an exercise for the reader. ;) --- diff --git a/ref-filter.c b/ref-filter.c index e1bcb4ca8a..b17eefed0d 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -20,6 +20,7 @@ #include "commit-slab.h" #include "commit-graph.h" #include "commit-reach.h" +#include "worktree.h" static struct ref_msg { const char *gone; @@ -114,6 +115,7 @@ static struct used_atom { } objectname; struct refname_atom refname; char *head; + struct string_list worktree_heads; } u; } *used_atom; static int used_atom_cnt, need_tagged, need_symref; @@ -420,6 +422,29 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a return 0; } +static int worktree_head_atom_parser(const struct ref_format *format, + struct used_atom *atom, + const char *arg, + struct strbuf *unused_err) +{ + struct worktree **worktrees = get_worktrees(0); + int i; + + string_list_init(&atom->u.worktree_heads, 1); + + for (i = 0; worktrees[i]; i++) { + if (worktrees[i]->head_ref) + string_list_append(&atom->u.worktree_heads, + worktrees[i]->head_ref); + } + + string_list_sort(&atom->u.worktree_heads); + + free_worktrees(worktrees); + return 0; + +} + static struct { const char *name; info_source source; @@ -460,6 +485,7 @@ static struct { { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser }, { "flag", SOURCE_NONE }, { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser }, + { "worktree", SOURCE_NONE, FIELD_STR, worktree_head_atom_parser }, { "color", SOURCE_NONE, FIELD_STR, color_atom_parser }, { "align", SOURCE_NONE, FIELD_STR, align_atom_parser }, { "end", SOURCE_NONE }, @@ -1588,6 +1614,13 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) else v->s = " "; continue; + } else if (!strcmp(name, "worktree")) { + if (string_list_has_string(&atom->u.worktree_heads, + ref->refname)) + v->s = "+"; + else + v->s = " "; + continue; } else if (starts_with(name, "align")) { v->handler = align_atom_handler; v->s = "";
Thanks for the feedback Peff. I actually agree with all your points. I'd considered an approach like what you proposed, but rejected it for the first iteration in an effort to keep scope limited and see what kind of feedback I'd get overall (like would people even want this?). This is a much better approach, and also gives a path for listing the worktree path in the verbose output. @Duy yea we can use a different color, maybe a darker shade of green. I saw plenty to choose from in the color list so I'll play around with it. It would definitely make it easier to distinguish at a glance which branch is checked out in the current worktree vs others. On Thu, Sep 27, 2018 at 11:17 AM Jeff King <peff@peff.net> wrote: > > On Thu, Sep 27, 2018 at 08:13:13AM -0700, Nickolai Belakovski wrote: > > > In order to more clearly display which branches are active, the output > > of git branch is modified to colorize branches checked out in any linked > > worktrees with the same color as the current branch. > > I think the goal makes sense. > > Do we want to limit this to git-branch, though? Ideally any output you > get from git-branch could be replicated with for-each-ref (or with > a custom "branch --format"). > > I.e., could we have a format in ref-filter that matches HEAD, but > returns a distinct symbol for a worktree HEAD? That would allow a few > things: > > - custom --formats for for-each-ref and branch could reuse the logic > > - we could show the symbol (in place of "*") even when color is not > enabled > > - it should be much faster if there are a lot of worktrees; your patch > does a linear if/else chain to look at each worktree, and it does it > in the format-language, which is much slower than actual C. :) > > Something like the patch below. I just picked "+" arbitrarily, but any > character would do (I avoided "*" just to make it visually distinct from > the current-worktree HEAD). I've left plugging this into git-branch's > default format as an exercise for the reader. ;) > > --- > diff --git a/ref-filter.c b/ref-filter.c > index e1bcb4ca8a..b17eefed0d 100644 > --- a/ref-filter.c > +++ b/ref-filter.c > @@ -20,6 +20,7 @@ > #include "commit-slab.h" > #include "commit-graph.h" > #include "commit-reach.h" > +#include "worktree.h" > > static struct ref_msg { > const char *gone; > @@ -114,6 +115,7 @@ static struct used_atom { > } objectname; > struct refname_atom refname; > char *head; > + struct string_list worktree_heads; > } u; > } *used_atom; > static int used_atom_cnt, need_tagged, need_symref; > @@ -420,6 +422,29 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a > return 0; > } > > +static int worktree_head_atom_parser(const struct ref_format *format, > + struct used_atom *atom, > + const char *arg, > + struct strbuf *unused_err) > +{ > + struct worktree **worktrees = get_worktrees(0); > + int i; > + > + string_list_init(&atom->u.worktree_heads, 1); > + > + for (i = 0; worktrees[i]; i++) { > + if (worktrees[i]->head_ref) > + string_list_append(&atom->u.worktree_heads, > + worktrees[i]->head_ref); > + } > + > + string_list_sort(&atom->u.worktree_heads); > + > + free_worktrees(worktrees); > + return 0; > + > +} > + > static struct { > const char *name; > info_source source; > @@ -460,6 +485,7 @@ static struct { > { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser }, > { "flag", SOURCE_NONE }, > { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser }, > + { "worktree", SOURCE_NONE, FIELD_STR, worktree_head_atom_parser }, > { "color", SOURCE_NONE, FIELD_STR, color_atom_parser }, > { "align", SOURCE_NONE, FIELD_STR, align_atom_parser }, > { "end", SOURCE_NONE }, > @@ -1588,6 +1614,13 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) > else > v->s = " "; > continue; > + } else if (!strcmp(name, "worktree")) { > + if (string_list_has_string(&atom->u.worktree_heads, > + ref->refname)) > + v->s = "+"; > + else > + v->s = " "; > + continue; > } else if (starts_with(name, "align")) { > v->handler = align_atom_handler; > v->s = "";
On Thu, Sep 27, 2018 at 11:39:26AM -0700, Nickolai Belakovski wrote: > Thanks for the feedback Peff. I actually agree with all your points. > I'd considered an approach like what you proposed, but rejected it for > the first iteration in an effort to keep scope limited and see what > kind of feedback I'd get overall (like would people even want this?). > This is a much better approach, and also gives a path for listing the > worktree path in the verbose output. Great. If you go that route, feel free to use whatever bits of my patch are useful. I tested it only by running "for-each-ref" once, so it might need some more help. Definitely tests and documentation at the least. :) -Peff
On Thu, Sep 27, 2018 at 02:17:08PM -0400, Jeff King wrote: > Do we want to limit this to git-branch, though? Ideally any output you > get from git-branch could be replicated with for-each-ref (or with > a custom "branch --format"). > > I.e., could we have a format in ref-filter that matches HEAD, but > returns a distinct symbol for a worktree HEAD? That would allow a few > things: I was going to suggest using dim green and green for elsewhere and here respectively, in a similar way how range-diff uses it to show different versions of the same diff. But if we're open to change how branches are displayed maybe a config option like branch.format (probably not the best name choice) that can be set to the 'for-each-ref --format' syntax would be way more flexible. e.g. branch.format='%(if:equals=+)...' I think the different symbol and dimmed color would be a nice addition, but I am leaning towards giving the user the ultimate choice on how they want to format their output. (Maybe with dimmed plus symbol as default). -- Cheers, Rafael Ascensão
On Thu, Sep 27, 2018 at 08:28:04PM +0100, Rafael Ascensão wrote: > On Thu, Sep 27, 2018 at 02:17:08PM -0400, Jeff King wrote: > > Do we want to limit this to git-branch, though? Ideally any output you > > get from git-branch could be replicated with for-each-ref (or with > > a custom "branch --format"). > > > > I.e., could we have a format in ref-filter that matches HEAD, but > > returns a distinct symbol for a worktree HEAD? That would allow a few > > things: > > I was going to suggest using dim green and green for elsewhere and here > respectively, in a similar way how range-diff uses it to show different > versions of the same diff. Yeah, I think that's reasonable, and would be enabled by the %(worktree) placeholder I showed. Just like we do something like: %(if)%(HEAD)%(then)* %(color:green)%(else) %(end) now, we could do: %(if)%(HEAD)%(then)* %(color:bold green) %(else)%(if)%(worktree)%(then)+ %(color:green) %(else) %(end)%(end) (respecting the user's color config, of course, rather than hard-coded colors). Trying that out, though, I'm not sure if we properly support nested if's. That might be a bug we have to fix first. > But if we're open to change how branches are displayed maybe a config > option like branch.format (probably not the best name choice) that can > be set to the 'for-each-ref --format' syntax would be way more flexible. We have that already, don't we? > I think the different symbol and dimmed color would be a nice addition, > but I am leaning towards giving the user the ultimate choice on how they > want to format their output. (Maybe with dimmed plus symbol as default). Definitely. -Peff
On Thu, Sep 27, 2018 at 03:35:59PM -0400, Jeff King wrote: > now, we could do: > > %(if)%(HEAD)%(then)* %(color:bold green) > %(else)%(if)%(worktree)%(then)+ %(color:green) > %(else) %(end)%(end) > > (respecting the user's color config, of course, rather than hard-coded > colors). > > Trying that out, though, I'm not sure if we properly support nested > if's. That might be a bug we have to fix first. Sorry, false alarm. I just had a typo in my format. This seems to work with the patch I posted earlier: git for-each-ref \ --format='%(if)%(HEAD)%(then)* %(color:bold green)%(else)%(if)%(worktree)%(then)+ %(color:green)%(else) %(end)%(end)%(refname)' \ refs/heads It sure would be nice if there was a way to insert line breaks without impacting the output. ;) -Peff
On Thu, Sep 27 2018, Rafael Ascensão wrote: > On Thu, Sep 27, 2018 at 02:17:08PM -0400, Jeff King wrote: >> Do we want to limit this to git-branch, though? Ideally any output you >> get from git-branch could be replicated with for-each-ref (or with >> a custom "branch --format"). >> >> I.e., could we have a format in ref-filter that matches HEAD, but >> returns a distinct symbol for a worktree HEAD? That would allow a few >> things: > > I was going to suggest using dim green and green for elsewhere and here > respectively, in a similar way how range-diff uses it to show different > versions of the same diff. It would be really useful to (just via E-Mail to start) itemize the colors we use in various places and what they mean. E.g. I thought green here made sense because in "diff" we show the old/new as red/green, so the branch you're on is "new" in the same sense, i.e. it's what your current state is. But maybe there's cases where that doesn't "rhyme" as it were.
Not to hijack my own thread, but FWIW git branch -r shows remote branches in red, but old/new status of a remote branch is ambiguous (could have new stuff, could be out of date). Also, git branch -vv shows remote tracking branches in blue. One could argue it should be red since git branch -r is in red. But yea, probably best to take this topic to its own thread. On Thu, Sep 27, 2018 at 1:02 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > > On Thu, Sep 27 2018, Rafael Ascensão wrote: > > > On Thu, Sep 27, 2018 at 02:17:08PM -0400, Jeff King wrote: > >> Do we want to limit this to git-branch, though? Ideally any output you > >> get from git-branch could be replicated with for-each-ref (or with > >> a custom "branch --format"). > >> > >> I.e., could we have a format in ref-filter that matches HEAD, but > >> returns a distinct symbol for a worktree HEAD? That would allow a few > >> things: > > > > I was going to suggest using dim green and green for elsewhere and here > > respectively, in a similar way how range-diff uses it to show different > > versions of the same diff. > > It would be really useful to (just via E-Mail to start) itemize the > colors we use in various places and what they mean. > > E.g. I thought green here made sense because in "diff" we show the > old/new as red/green, so the branch you're on is "new" in the same > sense, i.e. it's what your current state is. > > But maybe there's cases where that doesn't "rhyme" as it were.
On Thu, Sep 27, 2018 at 01:16:19PM -0700, Nickolai Belakovski wrote: > > Not to hijack my own thread, but FWIW git branch -r shows remote > branches in red, but old/new status of a remote branch is ambiguous > (could have new stuff, could be out of date). Also, git branch -vv > shows remote tracking branches in blue. One could argue it should be > red since git branch -r is in red. > For me remote branches being red means: they're here but you cannot write to them. They are like 'read-only/disabled' branches. Under this interpretation red makes sense. On Thu, Sep 27, 2018 at 9:02 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > E.g. I thought green here made sense because in "diff" we show the > old/new as red/green, so the branch you're on is "new" in the same > sense, i.e. it's what your current state is. > I still defend using green and dim green for this case. Because all these worktrees are in a sense active. They're checked out in some place. It's just the case that the particular one that we are in is probably more relevant than the others. -- Cheers Rafael Ascensão
Jeff King <peff@peff.net> writes: > On Thu, Sep 27, 2018 at 03:35:59PM -0400, Jeff King wrote: > >> now, we could do: >> >> %(if)%(HEAD)%(then)* %(color:bold green) >> %(else)%(if)%(worktree)%(then)+ %(color:green) >> %(else) %(end)%(end) >> >> (respecting the user's color config, of course, rather than hard-coded >> colors). >> >> Trying that out, though, I'm not sure if we properly support nested >> if's. That might be a bug we have to fix first. > > Sorry, false alarm. I just had a typo in my format. > > This seems to work with the patch I posted earlier: > > git for-each-ref \ > --format='%(if)%(HEAD)%(then)* %(color:bold green)%(else)%(if)%(worktree)%(then)+ %(color:green)%(else) %(end)%(end)%(refname)' \ > refs/heads > > It sure would be nice if there was a way to insert line breaks without > impacting the output. ;) > > -Peff I envy all of you who seem to have had a lot of fun while I was doing something else (which were rather boring but still needed precision--my least favorite kind of task X-<). The only comment I have is that I strongly suspect we will regret if we used an overly bland "worktree" to a rather narrrow "is this ref checked out in any worktree?" when we notice we want to learn other things that are related to "worktree". Other than that, very nicely done.
On Thu, Sep 27, 2018 at 03:35:59PM -0400, Jeff King wrote: > On Thu, Sep 27, 2018 at 08:28:04PM +0100, Rafael Ascensão wrote: > > But if we're open to change how branches are displayed maybe a config > > option like branch.format (probably not the best name choice) that can > > be set to the 'for-each-ref --format' syntax would be way more flexible. > > We have that already, don't we? > git branch has --format, but there's no way (at least to my knowledge) to define a value in gitconfig to be used by $git branch. Having branch --format available, making an alias is a possible route. (Either by wrapping branch --format or for-each-ref itself). But I was referring to changing the output of the default $git branch;
On Thu, Sep 27, 2018 at 02:22:49PM -0700, Junio C Hamano wrote: > The only comment I have is that I strongly suspect we will regret if > we used an overly bland "worktree" to a rather narrrow "is this ref > checked out in any worktree?" when we notice we want to learn other > things that are related to "worktree". Other than that, very nicely > done. Yeah, I should have mentioned that. %(worktree) was just a placeholder. Perhaps something like %(worktree-HEAD) would make more sense (the idea is that it is an extension of the existing %(HEAD) placeholder). Alternatively, %(HEAD) could return "*" or "+" depending on whether it's the current worktree head. That would mildly break an existing format like: %(if)%(HEAD)%(then) *%(color:green)%(end)%(refname) since it would start coloring worktree HEADs the same way. It would be rewritten as: %(if:equals=*)%(HEAD)%(then)...real HEAD... %(else)%(if:equals=+)%(HEAD)%(then)...worktree HEAD... %(else)...regular ref... %(end)%(end) I think that's perhaps nicer, but I'm not sure we want even such a minor regression. -Peff
On Thu, Sep 27, 2018 at 10:35:11PM +0100, Rafael Ascensão wrote: > git branch has --format, but there's no way (at least to my knowledge) > to define a value in gitconfig to be used by $git branch. Oh, you're right. I was thinking of the branch.sort we just added in v2.19. I agree that having branch.format (and a matching tag.format) would be useful. -Peff
Jeff King <peff@peff.net> writes: > Alternatively, %(HEAD) could return "*" or "+" depending on whether it's > the current worktree head. That would mildly break an existing format > like: > > %(if)%(HEAD)%(then) *%(color:green)%(end)%(refname) > > since it would start coloring worktree HEADs the same way. It would be > rewritten as: > > %(if:equals=*)%(HEAD)%(then)...real HEAD... > %(else)%(if:equals=+)%(HEAD)%(then)...worktree HEAD... > %(else)...regular ref... > %(end)%(end) > > I think that's perhaps nicer, but I'm not sure we want even such a minor > regression. I tend to think it is not worth having to worry about it by changing the meaning of %(HEAD) marking to save the effort to find a new token to fill that placeholder. Your %(worktreeHEAD) is good enough, I would think.
Hi Ævar, On Thu, 27 Sep 2018, Ævar Arnfjörð Bjarmason wrote: > On Thu, Sep 27 2018, Nickolai Belakovski wrote: > > > Will do re: screenshot when I get home, although it's pretty easy to > > imagine, the git branch output will have one other branch colored in green, > > bit without the asterisk (for one linked worktree) :) > > > > Also will do re: changing comments to /**/ (didn't know // was from C++, > > TIL) and I'll clean up the comments to remove some of the more obvious > > ones, but I'll try to keep a comment explaining the basic flow of creating > > a nest if statement to evaluate worktree refs for color. > > > > And yes, I copy/pasted into gmail. I was having trouble setting up > > send-email, but I think I may have it figured out now. Should I create a > > new thread with send-email? Or maybe reply to this one (I can do that by > > specifying the Message-ID to reply to right? > > You'd run git format-patch master..your-topic with > --subject-prefix="PATCH v2" and > --in-reply-to="<CAC05386q2iGoiJ_fRgwoOTF23exEN2D1+oh4VjajEvYQ58O1TQ@mail.gmail.com>". Then > it'll show up in reply to your v1. There is also a nice tutorial in https://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md#submit-your-patch (which, contrary to the location, is useful for non-Windows developers, too.) > You can also for an easier experience do this via GitGitGadget, see > https://github.com/gitgitgadget/gitgitgadget looking at its code it > seems to have some way to reference a Message-ID, but I don't know how > to trigger that. IIRC GitGitGadget has no facility yet to reply to any mail it did not generate itself (i.e. if you did not generate v1 using GitGitGadget, then it cannot generate a v2 that replies to the previous iteration). This might change at some stage, but I have other priorities for now. (Which should not stop any contributor from opening a PR to scratch their own favorite itch.) Ciao, Johannes > > > This is my first time using this workflow, so I appreciate your > > patience :) )? > > No worries, happy to help. > > > On Thu, Sep 27, 2018 at 8:33 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> > > wrote: > > > >> > >> On Thu, Sep 27 2018, Nickolai Belakovski wrote: > >> > >> > In order to more clearly display which branches are active, the output > >> > of git branch is modified to colorize branches checked out in any linked > >> > worktrees with the same color as the current branch. > >> > > >> > This is meant to simplify workflows related to worktree, particularly > >> > due to the limitations of not being able to check out the same branch in > >> > two worktrees and the inability to delete a branch checked out in a > >> > worktree. When performing branch operations like checkout and delete, it > >> > would be useful to know more readily if the branches in which the user > >> > is interested are already checked out in a worktree. > >> > > >> > The git worktree list command contains the relevant information, however > >> > this is a much less frquently used command than git branch. > >> > > >> > Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com> > >> > >> Sounds cool, b.t.w. would be neat-o to have some screenshot uploaded to > >> imgur or whatever just to skim what it looks like before/after. > >> > >> > diff --git a/builtin/branch.c b/builtin/branch.c > >> > index 4fc55c350..65b58ff7c 100644 > >> > --- a/builtin/branch.c > >> > +++ b/builtin/branch.c > >> > @@ -334,11 +334,36 @@ static char *build_format(struct ref_filter > >> > *filter, int maxwidth, const char *r > >> > struct strbuf local = STRBUF_INIT; > >> > struct strbuf remote = STRBUF_INIT; > >> > > >> > - strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else) > >> %s%%(end)", > >> > - branch_get_color(BRANCH_COLOR_CURRENT), > >> > - branch_get_color(BRANCH_COLOR_LOCAL)); > >> > - strbuf_addf(&remote, " %s", > >> > - branch_get_color(BRANCH_COLOR_REMOTE)); > >> > + // Prepend the current branch of this worktree with "* " and > >> > all other branches with " " > >> > >> > >> We use /* ... */ C comments, not C++-style // (well, it's in C now, but > >> not the ancient versions we need to support). > >> > >> It also seems all of this patch was copy/pasted into GMail or something, > >> it has wrapping and doesn't apply with "git am". > >> > >> Also most/all of these comments I'd say we could better do without, > >> i.e. the ones explaining basic code flow that's easy to see from the > >> code itself. > >> >
diff --git a/builtin/branch.c b/builtin/branch.c index 4fc55c350..65b58ff7c 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -334,11 +334,36 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r struct strbuf local = STRBUF_INIT; struct strbuf remote = STRBUF_INIT; - strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else) %s%%(end)", - branch_get_color(BRANCH_COLOR_CURRENT), - branch_get_color(BRANCH_COLOR_LOCAL)); - strbuf_addf(&remote, " %s", - branch_get_color(BRANCH_COLOR_REMOTE)); + // Prepend the current branch of this worktree with "* " and all other branches with " " + strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %%(else) %%(end)"); + // Prepend remote branches with two spaces + strbuf_addstr(&remote, " "); + if(want_color(branch_use_color)) { + // Create a nested if statement to evaluate if the current ref is equal to a HEAD ref from either + // the main or any linked worktrees. If so, color it CURRENT, otherwise color it LOCAL + struct strbuf color = STRBUF_INIT; + struct worktree **worktrees = get_worktrees(0); + int i; + for (i = 0; worktrees[i]; ++i) { + strbuf_addf(&color, "%%(if:equals=%s)%%(refname)%%(then)%s%%(else)", + worktrees[i]->head_ref, + branch_get_color(BRANCH_COLOR_CURRENT)); + } + // add one more check in the nested if-else to cover the detached HEAD state + strbuf_addf(&color, "%%(if)%%(HEAD)%%(then)%s%%(else)%s%%(end)", + branch_get_color(BRANCH_COLOR_CURRENT), + branch_get_color(BRANCH_COLOR_LOCAL)); + // close up the nested if-else + for (; i > 0; --i) { + strbuf_addf(&color, "%%(end)"); + } + free_worktrees(worktrees); + strbuf_addbuf(&local, &color); + strbuf_release(&color); + + strbuf_addf(&remote, "%s", + branch_get_color(BRANCH_COLOR_REMOTE)); + } if (filter->verbose) { struct strbuf obname = STRBUF_INIT; diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh index ee6787614..369a156c0 100755 --- a/t/t3203-branch-output.sh +++ b/t/t3203-branch-output.sh @@ -240,6 +240,27 @@ test_expect_success 'git branch --format option' ' test_i18ncmp expect actual ' +test_expect_success '"add" a worktree' ' + mkdir worktree_dir && + git worktree add -b master_worktree worktree_dir master +' + +cat >expect <<'EOF' +* <GREEN>(HEAD detached from fromtag)<RESET> + ambiguous<RESET> + branch-one<RESET> + branch-two<RESET> + master<RESET> + <GREEN>master_worktree<RESET> + ref-to-branch<RESET> -> branch-one + ref-to-remote<RESET> -> origin/branch-one +EOF +test_expect_success TTY 'worktree colors correct' ' + test_terminal git branch >actual.raw && + test_decode_color <actual.raw >actual && + test_cmp expect actual +' + test_expect_success "set up color tests" ' echo "<RED>master<RESET>" >expect.color && echo "master" >expect.bare &&
In order to more clearly display which branches are active, the output of git branch is modified to colorize branches checked out in any linked worktrees with the same color as the current branch. This is meant to simplify workflows related to worktree, particularly due to the limitations of not being able to check out the same branch in two worktrees and the inability to delete a branch checked out in a worktree. When performing branch operations like checkout and delete, it would be useful to know more readily if the branches in which the user is interested are already checked out in a worktree. The git worktree list command contains the relevant information, however this is a much less frquently used command than git branch. Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com> --- Notes: Travis CI results: https://travis-ci.org/nbelakovski/git/builds/432320949 builtin/branch.c | 35 ++++++++++++++++++++++++++++++----- t/t3203-branch-output.sh | 21 +++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-)