Re: [RFC/PATCH 07/11] branch: move 'current' check down to the presentation layer

Karthik Nayak [off-list ref] writes:

We check if given ref is the current branch in print_ref_list().  Move
this check to print_ref_item() where it is checked right before
printing.

This means that the '*' and the different color are coded in C, hence
it's not possible to mimick this using "git for-each-ref --format ...".

I do not consider this as blocking, but I think the ultimate goal should
be to allow this, so that all the goodies of "git branch" can be made
available to other ref-listing commands.

I actually prefered the old way: current is a Boolean that says
semantically "this is the current branch", and the Boolean is turned
into presentation directives in a separate piece of code.

I'd write

char c;
int current = 0;

...

if (...)
	current = 1;
...
case REF_DETACHED_HEAD:
	current = 1;
...

and keep the last hunk.

(IOW, turn current from a parameter into a local variable)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

On Tue, Jul 28, 2015 at 6:39 PM, Matthieu Moy
[off-list ref] wrote:

Karthik Nayak [off-list ref] writes:

quoted

We check if given ref is the current branch in print_ref_list().  Move
this check to print_ref_item() where it is checked right before
printing.

This means that the '*' and the different color are coded in C, hence
it's not possible to mimick this using "git for-each-ref --format ...".

I do not consider this as blocking, but I think the ultimate goal should
be to allow this, so that all the goodies of "git branch" can be made
available to other ref-listing commands.

Not sure what you mean here.

quoted

--- a/builtin/branch.c
+++ b/builtin/branch.c

@@ -534,9 +534,9 @@ static char *get_head_description(void)
 }

 static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
-                        int abbrev, int current, const char *remote_prefix)
+                        int abbrev, int detached, const char *remote_prefix)
 {
-     char c;
+     char c = ' ';
      int color;
      struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
      const char *prefix = "";

@@ -547,7 +547,11 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,

      switch (item->kind) {
      case REF_LOCAL_BRANCH:
-             color = BRANCH_COLOR_LOCAL;
+             if (!detached && !strcmp(item->name, head)) {
+                     color = BRANCH_COLOR_CURRENT;
+                     c = '*';
+             } else
+                     color = BRANCH_COLOR_LOCAL;
              break;
      case REF_REMOTE_BRANCH:
              color = BRANCH_COLOR_REMOTE;

@@ -556,18 +560,13 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
      case REF_DETACHED_HEAD:
              color = BRANCH_COLOR_CURRENT;
              desc = get_head_description();
+             c = '*';
              break;
      default:
              color = BRANCH_COLOR_PLAIN;
              break;
      }

-     c = ' ';
-     if (current) {
-             c = '*';
-             color = BRANCH_COLOR_CURRENT;
-     }

I actually prefered the old way: current is a Boolean that says
semantically "this is the current branch", and the Boolean is turned
into presentation directives in a separate piece of code.

I'd write

char c;
int current = 0;

...

if (...)
        current = 1;
...
case REF_DETACHED_HEAD:
        current = 1;
...

and keep the last hunk.

(IOW, turn current from a parameter into a local variable)

Thanks will do this.

-- 
Regards,
Karthik Nayak

On Tue, Jul 28, 2015 at 1:12 PM, Karthik Nayak [off-list ref] wrote:

On Tue, Jul 28, 2015 at 6:39 PM, Matthieu Moy
[off-list ref] wrote:

quoted

Karthik Nayak [off-list ref] writes:

quoted

We check if given ref is the current branch in print_ref_list().  Move
this check to print_ref_item() where it is checked right before
printing.

This means that the '*' and the different color are coded in C, hence
it's not possible to mimick this using "git for-each-ref --format ...".

I do not consider this as blocking, but I think the ultimate goal should
be to allow this, so that all the goodies of "git branch" can be made
available to other ref-listing commands.

Not sure what you mean here.

He means to make sure that any feature that was in tag, branch,
for-each-ref, etc should be available as part of ref-filter and in all
of them

Maybe he misunderstood code or maybe you misunderstood the comment here?

Regards,
Jake

On Wed, Jul 29, 2015 at 6:16 AM, Jacob Keller [off-list ref] wrote:

On Tue, Jul 28, 2015 at 1:12 PM, Karthik Nayak [off-list ref] wrote:

quoted

On Tue, Jul 28, 2015 at 6:39 PM, Matthieu Moy
[off-list ref] wrote:

quoted

Karthik Nayak [off-list ref] writes:

quoted

We check if given ref is the current branch in print_ref_list().  Move
this check to print_ref_item() where it is checked right before
printing.

This means that the '*' and the different color are coded in C, hence
it's not possible to mimick this using "git for-each-ref --format ...".

I do not consider this as blocking, but I think the ultimate goal should
be to allow this, so that all the goodies of "git branch" can be made
available to other ref-listing commands.

Not sure what you mean here.

He means to make sure that any feature that was in tag, branch,
for-each-ref, etc should be available as part of ref-filter and in all
of them

Maybe he misunderstood code or maybe you misunderstood the comment here?

Regards,
Jake

Ah, I didn't quite understand the first time. Thanks :)

-- 
Regards,
Karthik Nayak