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

Re: [PATCH v2 00/10] port branch.c to use ref-filter's printing options

From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date
Oct 8, 2015, 12:27 UTC
Message-ID
<vpqr3l5zgst.fsf@grenoble-inp.fr>
In-Reply-To
<1444295885-1657-1-git-send-email-Karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 7 quoted lines
> +An example to show the usage of %(if)...%(then)...%(else)...%(end).
> +This prefixes the current branch with a star.
> +
> +------------
> +#!/bin/sh
> +
> +git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  %(end)%(refname:short)" refs/heads/

I don't think the #!/bin/sh adds any value here. Just the 'git for-each-ref' line is sufficient IMHO.

> +An example to show the usage of %(if)...%(then)...%(end).
> +This adds a red color to authorname, if present

I don't think this is such a good example. %(color:red)%(authorname)%(color:reset) just works even if %(authorname) is empty.

A better example would be
git for-each-ref --format='%(if)%(authorname)%(then)Authored by %(authorname)%(end)'
which avoids writting "Authored by " with no author.
> -static int is_empty(const char * s){
> +static int is_empty(const char *s){

You still have the { on the declaration line, it should be on the next line.

Show 9 quoted lines
> @@ -309,10 +311,14 @@ static int is_empty(const char * s){
>  static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
>  {
>  	struct ref_formatting_stack *cur = state->stack;
> -	struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
> +	struct if_then_else *if_then_else = NULL;
>  
> +	if (cur->at_end == if_then_else_handler)
> +		if_then_else = (struct if_then_else *)cur->at_end_data;

OK, now the cast is safe since at_end_data has to be of type struct if_then_else * if at_end is if_then_else_handler.

> +				unsigned int nobracket = 0;
> +
> +				if (!strcmp(valp, ",nobracket"))
> +					nobracket = 1;

The code to parse comma-separated values is different here and elsewhere. I'd rather have the same code (possibly factored into a helper function), both to get consistent behavior (you're not allowing %(upstream:nobracket,track) for example, right?) and consistent code.

Show 8 quoted lines
>  				if (!num_ours && !num_theirs)
>  					v->s = "";
>  				else if (!num_ours) {
> -					sprintf(buf, "[behind %d]", num_theirs);
> +					if (nobracket)
> +						sprintf(buf, "behind %d", num_theirs);
> +					else
> +						sprintf(buf, "[behind %d]", num_theirs);

Perhaps use sprintf(buf, "%sbehind %d%s", obracket, num_their, cbracket) unconditionnally, and set obracket = "" or obracket = "[" once and for all when you test for "nobracket" above. This avoids these "if (nobracket)" spread accross the code, but at the price of extra %s in the format strings.

Show 14 quoted lines
> @@ -1170,6 +1173,29 @@ static void populate_value(struct ref_array_item *ref)
>  				else
>  					v->s = "<>";
>  				continue;
> +			} else if (!strcmp(formatp, "dir") &&
> +				   (starts_with(name, "refname"))) {
> +				const char *sp, *ep, *tmp;
> +
> +				sp = tmp = ref->refname;
> +				/*  Obtain refs/foo/bar/ from refname refs/foo/bar/abc */
> +				do {
> +					ep = tmp;
> +					tmp = strchr(ep + 1, '/');
> +				} while (tmp);

To look for the last occurence of '/' you can also use strrchr(). Doesn't it do what you want here?

Show 18 quoted lines
> --- a/t/t6040-tracking-info.sh
> +++ b/t/t6040-tracking-info.sh
> @@ -58,11 +58,11 @@ test_expect_success 'branch -v' '
>  '
>  
>  cat >expect <<\EOF
> -b1 [origin/master] [ahead 1, behind 1] d
> -b2 [origin/master] [ahead 1, behind 1] d
> -b3 [origin/master] [behind 1] b
> -b4 [origin/master] [ahead 2] f
> -b5 [brokenbase] [gone] g
> +b1 [origin/master: ahead 1, behind 1] d
> +b2 [origin/master: ahead 1, behind 1] d
> +b3 [origin/master: behind 1] b
> +b4 [origin/master: ahead 2] f
> +b5 [brokenbase: gone] g
>  b6 [origin/master] c
>  EOF
Cool!

I didn't go through the patches themselves, but modulo my remarks above the interdiff looks good. Thanks.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Previous: Karthik NayakNext: Karthik Nayak
Message 44 of 66 in “port branch.c to use ref-filter's printing options”
  1. 00/10 port branch.c to use ref-filter's printing optionsKarthik Nayak, Oct 8, 2015
  2. 01/10 ref-filter: implement %(if), %(then), and %(else) atomsKarthik Nayak, Oct 8, 2015
  3. Matthieu MoyOct 8, 2015
  4. Karthik NayakOct 10, 2015
  5. Matthieu MoyOct 8, 2015
  6. Karthik NayakOct 10, 2015
  7. 02/10 ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)Karthik Nayak, Oct 8, 2015
  8. Matthieu MoyOct 8, 2015
  9. Karthik NayakOct 10, 2015
  10. 03/10 ref-filter: add support for %(refname:dir) and %(refname:base)Karthik Nayak, Oct 8, 2015
  11. 04/10 ref-filter: modify "%(objectname:short)" to take lengthKarthik Nayak, Oct 8, 2015
  12. Matthieu MoyOct 8, 2015
  13. Karthik NayakOct 10, 2015
  14. Matthieu MoyOct 11, 2015
  15. Karthik NayakOct 11, 2015
  16. 05/10 ref-filter: adopt get_head_description() from branch.cKarthik Nayak, Oct 8, 2015
  17. Matthieu MoyOct 8, 2015
  18. Karthik NayakOct 10, 2015
  19. 06/10 ref-filter: introduce format_ref_array_item()Karthik Nayak, Oct 8, 2015
  20. 07/10 ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreamsKarthik Nayak, Oct 8, 2015
  21. Matthieu MoyOct 8, 2015
  22. Karthik NayakOct 10, 2015
  23. Matthieu MoyOct 11, 2015
  24. 1/3 fixup: use xstrfmt instead of fixed-size buf + sprintf + xstrdupMatthieu Moy, Oct 11, 2015
  25. 2/3 ref-filter: allow porcelain to translate messages in the outputMatthieu Moy, Oct 11, 2015
  26. Karthik NayakOct 11, 2015
  27. 3/3 branch, tag: use porcelain outputMatthieu Moy, Oct 11, 2015
  28. Junio C HamanoOct 12, 2015
  29. Karthik NayakOct 12, 2015
  30. Karthik NayakOct 11, 2015
  31. Matthieu MoyOct 11, 2015
  32. Karthik NayakOct 11, 2015
  33. 08/10 ref-filter: add support for %(upstream:track,nobracket)Karthik Nayak, Oct 8, 2015
  34. Matthieu MoyOct 8, 2015
  35. Matthieu MoyOct 8, 2015
  36. Karthik NayakOct 13, 2015
  37. Matthieu MoyOct 13, 2015
  38. Junio C HamanoOct 13, 2015
  39. Karthik NayakOct 14, 2015
  40. 09/10 branch: use ref-filter printing APIsKarthik Nayak, Oct 8, 2015
  41. Junio C HamanoOct 13, 2015
  42. Karthik NayakOct 13, 2015
  43. 10/10 branch: implement '--format' optionKarthik Nayak, Oct 8, 2015
  44. Matthieu MoyOct 8, 2015
  45. Karthik NayakOct 8, 2015
  46. Karthik NayakOct 8, 2015
  47. Matthieu MoyOct 8, 2015
  48. Karthik NayakOct 8, 2015
  49. Matthieu MoyOct 8, 2015
  50. Junio C HamanoOct 8, 2015
  51. Matthieu MoyOct 9, 2015
  52. Junio C HamanoOct 9, 2015
  53. Karthik NayakOct 11, 2015
  54. Matthieu MoyOct 11, 2015
  55. Karthik NayakOct 11, 2015
  56. Matthieu MoyOct 11, 2015
  57. Karthik NayakOct 11, 2015
  58. Junio C HamanoOct 12, 2015
  59. Karthik NayakOct 12, 2015
  60. Matthieu MoyOct 12, 2015
  61. Junio C HamanoOct 12, 2015
  62. Matthieu MoyOct 12, 2015
  63. Karthik NayakOct 18, 2015
  64. Junio C HamanoOct 19, 2015
  65. Matthieu MoyOct 19, 2015
  66. Karthik NayakOct 20, 2015

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.