Re: [PATCH 02/10] branch: refactor width computation
On Tue, Aug 11, 2015 at 7:28 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak karthik@gmail.com wrote: 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@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.) Ah right, will put that after the check :D Yea, that seems out of place. Thanks -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] branch: refactor width computation
On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak karthik@gmail.com wrote: 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@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.) - 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@gmail.com wrote: From: Karthik Nayak karthik@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 p...@peff.net Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@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
[PATCH 02/10] branch: refactor width computation
From: Karthik Nayak karthik@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 p...@peff.net Mentored-by: Christian Couder christian.cou...@gmail.com Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Karthik Nayak karthik@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