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

Re: [PATCH v9 19/20] branch: use ref-filter printing APIs

From
Karthik Nayak <karthik.188@gmail.com>
Date
Dec 28, 2016, 09:40 UTC
Message-ID
<CAOLa=ZTPMbhzv_9mLv63UvPu__JqwQrpd2d8ogXuz5wNKb70Dw@mail.gmail.com>
In-Reply-To
<xmqqinq56phu.fsf@gitster.mtv.corp.google.com>
On Wed, Dec 28, 2016 at 2:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 43 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>  static char branch_colors[][COLOR_MAXLEN] = {
>> -     GIT_COLOR_RESET,
>> -     GIT_COLOR_NORMAL,       /* PLAIN */
>> -     GIT_COLOR_RED,          /* REMOTE */
>> -     GIT_COLOR_NORMAL,       /* LOCAL */
>> -     GIT_COLOR_GREEN,        /* CURRENT */
>> -     GIT_COLOR_BLUE,         /* UPSTREAM */
>> +     "%(color:reset)",
>> +     "%(color:reset)",       /* PLAIN */
>> +     "%(color:red)",         /* REMOTE */
>> +     "%(color:reset)",       /* LOCAL */
>> +     "%(color:green)",       /* CURRENT */
>> +     "%(color:blue)",        /* UPSTREAM */
>>  };
>
> The contents of this table is no longer tied to COLOR_MAXLEN.  The
> above entries used by default happen to be shorter, but it is
> misleading.
>
>         static const char *branch_colors[] = {
>         "%(color:reset)",
>         ...
>         };
>
> perhaps?
>
> More importantly, does this re-definition of the branch_colors[]
> work with custom colors filled in git_branch_config() by calling
> e.g. color_parse("red", branch_colors[BRANCH_COLOR_REMOTE]), where
> "red" and "remote" come from an existing configuration file?
>
>         [color "branch"]
>                 remote = red
>
> It obviously would not work if you changed the type of branch_colors[]
> because the color_parse() wants the caller to have allocated space
> of COLOR_MAXLEN.
>
> But if filling these ANSI escape sequence into the format works OK,
> then doesn't it tell us that we do not need to have this change to
> use "%(color:reset)" etc. as the new default values?

Good point, this would overwrite the existing configuration based setup existing in builtin/branch.c.

I think it'd make sense to use the existing branch_colors[] definition without any changes. That's mean that instead of using %(color:<color>). We hard code the colors by calling branch_get_color(). This is ok with me since, users who which to have their own formats will anyways use --format option.

-- 
Regards,
Karthik Nayak
Previous: Junio C HamanoNext: Karthik Nayak
Message 33 of 34 in “port branch.c to use ref-filter's printing options”
  1. 00/20 port branch.c to use ref-filter's printing optionsKarthik Nayak, Dec 27, 2016
  2. 01/20 ref-filter: implement %(if), %(then), and %(else) atomsKarthik Nayak, Dec 27, 2016
  3. Junio C HamanoDec 27, 2016
  4. Karthik NayakDec 28, 2016
  5. 03/20 ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)Karthik Nayak, Dec 27, 2016
  6. Junio C HamanoDec 27, 2016
  7. Karthik NayakDec 28, 2016
  8. 02/20 ref-filter: include reference to 'used_atom' within 'atom_value'Karthik Nayak, Dec 27, 2016
  9. Junio C HamanoDec 27, 2016
  10. Karthik NayakDec 28, 2016
  11. 05/20 ref-filter: move get_head_description() from branch.cKarthik Nayak, Dec 27, 2016
  12. 04/20 ref-filter: modify "%(objectname:short)" to take lengthKarthik Nayak, Dec 27, 2016
  13. 06/20 ref-filter: introduce format_ref_array_item()Karthik Nayak, Dec 27, 2016
  14. 07/20 ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreamsKarthik Nayak, Dec 27, 2016
  15. 08/20 ref-filter: add support for %(upstream:track,nobracket)Karthik Nayak, Dec 27, 2016
  16. 10/20 ref-filter: introduce refname_atom_parser_internal()Karthik Nayak, Dec 27, 2016
  17. 09/20 ref-filter: make "%(symref)" atom work with the ':short' modifierKarthik Nayak, Dec 27, 2016
  18. 12/20 ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()Karthik Nayak, Dec 27, 2016
  19. 11/20 ref-filter: introduce refname_atom_parser()Karthik Nayak, Dec 27, 2016
  20. Junio C HamanoDec 27, 2016
  21. Karthik NayakDec 28, 2016
  22. 14/20 ref-filter: Do not abruptly die when using the 'lstrip=<N>' optionKarthik Nayak, Dec 27, 2016
  23. 13/20 ref-filter: rename the 'strip' option to 'lstrip'Karthik Nayak, Dec 27, 2016
  24. 15/20 ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'Karthik Nayak, Dec 27, 2016
  25. Junio C HamanoDec 27, 2016
  26. Jacob KellerDec 28, 2016
  27. Karthik NayakDec 28, 2016
  28. 16/20 ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnamesKarthik Nayak, Dec 27, 2016
  29. 17/20 ref-filter: allow porcelain to translate messages in the outputKarthik Nayak, Dec 27, 2016
  30. 18/20 branch, tag: use porcelain outputKarthik Nayak, Dec 27, 2016
  31. 19/20 branch: use ref-filter printing APIsKarthik Nayak, Dec 27, 2016
  32. Junio C HamanoDec 27, 2016
  33. Karthik NayakDec 28, 2016
  34. 20/20 branch: implement '--format' optionKarthik Nayak, Dec 27, 2016

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.