Re: [PATCH] branch: colorize branches checked out in a linked working tree the same way as the current branch is colorized

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx> 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 = "";



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux