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

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

 



On Tue, Jul 28, 2015 at 6:39 PM, Matthieu Moy
<Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> 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.
>

Not sure what you mean here.

>> --- 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]