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

2015-08-11 Thread Karthik Nayak
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

2015-08-10 Thread Eric Sunshine
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

2015-08-04 Thread Karthik Nayak
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