git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 02/10] branch: refactor width computation

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Aug 11, 2015, 01:58 UTC
Message-ID
<CAPig+cRHvkAEM1ayVc5nj6RCih52x1+Bn9YTQPV93ej=3arK2Q@mail.gmail.com>
In-Reply-To
<1438693282-15516-2-git-send-email-Karthik.188@gmail.com>
On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 27 quoted lines
> Remove unnecessary variables from ref_list and ref_item which were
> used for width computation. This is to make ref_item similar to
> ref-filter's ref_array_item. This will ensure a smooth port of
> branch.c to use ref-filter APIs in further patches.
>
> Previously the maxwidth was computed when inserting the refs into the
> ref_list. Now, we obtain the entire ref_list and then compute
> maxwidth.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 4fc8beb..b058b74 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -557,16 +552,21 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
> -static int calc_maxwidth(struct ref_list *refs)
> +static int calc_maxwidth(struct ref_list *refs, int remote_bonus)
>  {
> -       int i, w = 0;
> +       int i, max = 0;
>         for (i = 0; i < refs->index; i++) {
> +               struct ref_item *it = &refs->list[i];
> +               int w = utf8_strwidth(it->name);
> +
>                 if (refs->list[i].ignore)
>                         continue;
Nit: Unnecessary work. You compute the width and then immediately
throw it away when 'ignore' is true.

Also, you use 'it' elsewhere rather than 'refs->list[i]' but not this line, which makes it seem out of place. I would have expected to see:

    if (it->ignore)
        continue;
(Despite the noisier diff, the end result will be more consistent.)
Show 10 quoted lines
> -               if (refs->list[i].width > w)
> -                       w = refs->list[i].width;
> +               if (it->kind == REF_REMOTE_BRANCH)
> +                       w += remote_bonus;
> +               if (w > max)
> +                       max = w;
>         }
> -       return w;
> +       return max;
>  }
On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 185 quoted lines
> From: Karthik Nayak <karthik.188@gmail.com>
>
> Remove unnecessary variables from ref_list and ref_item which were
> used for width computation. This is to make ref_item similar to
> ref-filter's ref_array_item. This will ensure a smooth port of
> branch.c to use ref-filter APIs in further patches.
>
> Previously the maxwidth was computed when inserting the refs into the
> ref_list. Now, we obtain the entire ref_list and then compute
> maxwidth.
>
> Based-on-patch-by: Jeff King <peff@peff.net>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  builtin/branch.c | 61 +++++++++++++++++++++++++++++---------------------------
>  1 file changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 4fc8beb..b058b74 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -282,14 +282,14 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>  struct ref_item {
>         char *name;
>         char *dest;
> -       unsigned int kind, width;
> +       unsigned int kind;
>         struct commit *commit;
>         int ignore;
>  };
>
>  struct ref_list {
>         struct rev_info revs;
> -       int index, alloc, maxwidth, verbose, abbrev;
> +       int index, alloc, verbose, abbrev;
>         struct ref_item *list;
>         struct commit_list *with_commit;
>         int kinds;
> @@ -386,15 +386,8 @@ static int append_ref(const char *refname, const struct object_id *oid, int flag
>         newitem->name = xstrdup(refname);
>         newitem->kind = kind;
>         newitem->commit = commit;
> -       newitem->width = utf8_strwidth(refname);
>         newitem->dest = resolve_symref(orig_refname, prefix);
>         newitem->ignore = 0;
> -       /* adjust for "remotes/" */
> -       if (newitem->kind == REF_REMOTE_BRANCH &&
> -           ref_list->kinds != REF_REMOTE_BRANCH)
> -               newitem->width += 8;
> -       if (newitem->width > ref_list->maxwidth)
> -               ref_list->maxwidth = newitem->width;
>
>         return 0;
>  }
> @@ -505,11 +498,12 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item,
>  }
>
>  static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
> -                          int abbrev, int current, char *prefix)
> +                          int abbrev, int current, const char *remote_prefix)
>  {
>         char c;
>         int color;
>         struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
> +       const char *prefix = "";
>
>         if (item->ignore)
>                 return;
> @@ -520,6 +514,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>                 break;
>         case REF_REMOTE_BRANCH:
>                 color = BRANCH_COLOR_REMOTE;
> +               prefix = remote_prefix;
>                 break;
>         default:
>                 color = BRANCH_COLOR_PLAIN;
> @@ -557,16 +552,21 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>         strbuf_release(&out);
>  }
>
> -static int calc_maxwidth(struct ref_list *refs)
> +static int calc_maxwidth(struct ref_list *refs, int remote_bonus)
>  {
> -       int i, w = 0;
> +       int i, max = 0;
>         for (i = 0; i < refs->index; i++) {
> +               struct ref_item *it = &refs->list[i];
> +               int w = utf8_strwidth(it->name);
> +
>                 if (refs->list[i].ignore)
>                         continue;
> -               if (refs->list[i].width > w)
> -                       w = refs->list[i].width;
> +               if (it->kind == REF_REMOTE_BRANCH)
> +                       w += remote_bonus;
> +               if (w > max)
> +                       max = w;
>         }
> -       return w;
> +       return max;
>  }
>
>  static char *get_head_description(void)
> @@ -600,21 +600,18 @@ static char *get_head_description(void)
>         return strbuf_detach(&desc, NULL);
>  }
>
> -static void show_detached(struct ref_list *ref_list)
> +static void show_detached(struct ref_list *ref_list, int maxwidth)
>  {
>         struct commit *head_commit = lookup_commit_reference_gently(head_sha1, 1);
>
>         if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
>                 struct ref_item item;
>                 item.name = get_head_description();
> -               item.width = utf8_strwidth(item.name);
>                 item.kind = REF_LOCAL_BRANCH;
>                 item.dest = NULL;
>                 item.commit = head_commit;
>                 item.ignore = 0;
> -               if (item.width > ref_list->maxwidth)
> -                       ref_list->maxwidth = item.width;
> -               print_ref_item(&item, ref_list->maxwidth, ref_list->verbose, ref_list->abbrev, 1, "");
> +               print_ref_item(&item, maxwidth, ref_list->verbose, ref_list->abbrev, 1, "");
>                 free(item.name);
>         }
>  }
> @@ -624,6 +621,16 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
>         int i;
>         struct append_ref_cb cb;
>         struct ref_list ref_list;
> +       int maxwidth = 0;
> +       const char *remote_prefix = "";
> +
> +       /*
> +        * If we are listing more than just remote branches,
> +        * then remote branches will have a "remotes/" prefix.
> +        * We need to account for this in the width.
> +        */
> +       if (kinds != REF_REMOTE_BRANCH)
> +               remote_prefix = "remotes/";
>
>         memset(&ref_list, 0, sizeof(ref_list));
>         ref_list.kinds = kinds;
> @@ -667,26 +674,22 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
>                         clear_commit_marks(item->commit, ALL_REV_FLAGS);
>                 }
>                 clear_commit_marks(filter, ALL_REV_FLAGS);
> -
> -               if (verbose)
> -                       ref_list.maxwidth = calc_maxwidth(&ref_list);
>         }
> +       if (verbose)
> +               maxwidth = calc_maxwidth(&ref_list, strlen(remote_prefix));
>
>         qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
>
>         detached = (detached && (kinds & REF_LOCAL_BRANCH));
>         if (detached && match_patterns(pattern, "HEAD"))
> -               show_detached(&ref_list);
> +               show_detached(&ref_list, maxwidth);
>
>         for (i = 0; i < ref_list.index; i++) {
>                 int current = !detached &&
>                         (ref_list.list[i].kind == REF_LOCAL_BRANCH) &&
>                         !strcmp(ref_list.list[i].name, head);
> -               char *prefix = (kinds != REF_REMOTE_BRANCH &&
> -                               ref_list.list[i].kind == REF_REMOTE_BRANCH)
> -                               ? "remotes/" : "";
> -               print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose,
> -                              abbrev, current, prefix);
> +               print_ref_item(&ref_list.list[i], maxwidth, verbose,
> +                              abbrev, current, remote_prefix);
>         }
>
>         free_ref_list(&ref_list);
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous: Karthik NayakNext: Karthik Nayak
Message 12 of 27 in “Port branch.c to ref-filter.”
  1. 0/10 Port branch.c to ref-filter.Karthik Nayak, Aug 4, 2015
  2. 01/10 ref-filter: add option to filter only branchesKarthik Nayak, Aug 4, 2015
  3. Junio C HamanoAug 11, 2015
  4. Karthik NayakAug 13, 2015
  5. Karthik NayakAug 13, 2015
  6. Karthik NayakAug 13, 2015
  7. Junio C HamanoAug 14, 2015
  8. Karthik NayakAug 14, 2015
  9. Matthieu MoyAug 13, 2015
  10. Karthik NayakAug 13, 2015
  11. 02/10 branch: refactor width computationKarthik Nayak, Aug 4, 2015
  12. Eric SunshineAug 11, 2015
  13. Karthik NayakAug 11, 2015
  14. 03/10 branch: bump get_head_description() to the topKarthik Nayak, Aug 4, 2015
  15. Eric SunshineAug 11, 2015
  16. Karthik NayakAug 11, 2015
  17. 04/10 branch: roll show_detached HEAD into regular ref_listKarthik Nayak, Aug 4, 2015
  18. Eric SunshineAug 11, 2015
  19. Karthik NayakAug 11, 2015
  20. 05/10 branch: move 'current' check down to the presentation layerKarthik Nayak, Aug 4, 2015
  21. 06/10 branch: drop non-commit error reportingKarthik Nayak, Aug 4, 2015
  22. 07/10 branch.c: use 'ref-filter' data structuresKarthik Nayak, Aug 4, 2015
  23. 08/10 branch.c: use 'ref-filter' APIsKarthik Nayak, Aug 4, 2015
  24. 09/10 branch: add '--points-at' optionKarthik Nayak, Aug 4, 2015
  25. Karthik NayakAug 4, 2015
  26. Junio C HamanoAug 5, 2015
  27. Karthik NayakAug 7, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.