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

Re: [PATCH v7 11/17] ref-filter: introduce symref_atom_parser() and refname_atom_parser()

From
Jacob Keller <jacob.keller@gmail.com>
Date
Nov 8, 2016, 23:52 UTC
Message-ID
<CA+P7+xrHRMZemNfo3gjJryBRR5b4T8VT3t9_CBE5hJJfv+D3PQ@mail.gmail.com>
In-Reply-To
<20161108201211.25213-12-Karthik.188@gmail.com>
On Tue, Nov 8, 2016 at 12:12 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 10 quoted lines
> From: Karthik Nayak <karthik.188@gmail.com>
>
> Using refname_atom_parser_internal(), introduce symref_atom_parser() and
> refname_atom_parser() which will parse the atoms %(symref) and
> %(refname) respectively. Store the parsed information into the
> 'used_atom' structure based on the modifiers used along with the atoms.
>
> Now the '%(symref)' atom supports the ':strip' atom modifier. Update the
> Documentation and tests to reflect this.
>

One minor nit is that the first part is actually identical so I wonder if it's worth having two separate functions?

Thanks, Jake

Show 42 quoted lines
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
> ---
>  Documentation/git-for-each-ref.txt |  5 +++
>  ref-filter.c                       | 78 ++++++++++++++++++++++----------------
>  t/t6300-for-each-ref.sh            |  9 +++++
>  3 files changed, 59 insertions(+), 33 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 3953431..a669a32 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -166,6 +166,11 @@ if::
>         the value between the %(if:...) and %(then) atoms with the
>         given string.
>
> +symref::
> +       The ref which the given symbolic ref refers to. If not a
> +       symbolic ref, nothing is printed. Respects the `:short` and
> +       `:strip` options in the same way as `refname` above.
> +
>  In addition to the above, for commit and tag objects, the header
>  field names (`tree`, `parent`, `object`, `type`, and `tag`) can
>  be used to specify the value in the header field.
> diff --git a/ref-filter.c b/ref-filter.c
> index aad537d..f1d27b5 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -176,6 +176,16 @@ static void objectname_atom_parser(struct used_atom *atom, const char *arg)
>                 die(_("unrecognized %%(objectname) argument: %s"), arg);
>  }
>
> +static void symref_atom_parser(struct used_atom *atom, const char *arg)
> +{
> +       return refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
> +}
> +
> +static void refname_atom_parser(struct used_atom *atom, const char *arg)
> +{
> +       return refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
> +}
> +

What's the reasoning for using separate functions here if they are exactly identical except for name? Do we intend to add separate options for this? I don't really have a problem with separate functions here since it helps avoid confusion but they are identical otherwise...

Show 154 quoted lines
>  static align_type parse_align_position(const char *s)
>  {
>         if (!strcmp(s, "right"))
> @@ -244,7 +254,7 @@ static struct {
>         cmp_type cmp_type;
>         void (*parser)(struct used_atom *atom, const char *arg);
>  } valid_atom[] = {
> -       { "refname" },
> +       { "refname" , FIELD_STR, refname_atom_parser },
>         { "objecttype" },
>         { "objectsize", FIELD_ULONG },
>         { "objectname", FIELD_STR, objectname_atom_parser },
> @@ -273,7 +283,7 @@ static struct {
>         { "contents", FIELD_STR, contents_atom_parser },
>         { "upstream", FIELD_STR, remote_ref_atom_parser },
>         { "push", FIELD_STR, remote_ref_atom_parser },
> -       { "symref" },
> +       { "symref", FIELD_STR, symref_atom_parser },
>         { "flag" },
>         { "HEAD" },
>         { "color", FIELD_STR, color_atom_parser },
> @@ -1058,21 +1068,16 @@ static inline char *copy_advance(char *dst, const char *src)
>         return dst;
>  }
>
> -static const char *strip_ref_components(const char *refname, const char *nr_arg)
> +static const char *strip_ref_components(const char *refname, unsigned int len)
>  {
> -       char *end;
> -       long nr = strtol(nr_arg, &end, 10);
> -       long remaining = nr;
> +       long remaining = len;
>         const char *start = refname;
>
> -       if (nr < 1 || *end != '\0')
> -               die(_(":strip= requires a positive integer argument"));
> -
>         while (remaining) {
>                 switch (*start++) {
>                 case '\0':
> -                       die(_("ref '%s' does not have %ld components to :strip"),
> -                           refname, nr);
> +                       die(_("ref '%s' does not have %ud components to :strip"),
> +                           refname, len);
>                 case '/':
>                         remaining--;
>                         break;
> @@ -1081,6 +1086,16 @@ static const char *strip_ref_components(const char *refname, const char *nr_arg)
>         return start;
>  }
>
> +static const char *show_ref(struct refname_atom *atom, const char *refname)
> +{
> +       if (atom->option == R_SHORT)
> +               return shorten_unambiguous_ref(refname, warn_ambiguous_refs);
> +       else if (atom->option == R_STRIP)
> +               return strip_ref_components(refname, atom->strip);
> +       else
> +               return refname;
> +}
> +
>  static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
>                                     struct branch *branch, const char **s)
>  {
> @@ -1153,6 +1168,21 @@ char *get_head_description(void)
>         return strbuf_detach(&desc, NULL);
>  }
>
> +static const char *get_symref(struct used_atom *atom, struct ref_array_item *ref)
> +{
> +       if (!ref->symref)
> +               return "";
> +       else
> +               return show_ref(&atom->u.refname, ref->symref);
> +}
> +
> +static const char *get_refname(struct used_atom *atom, struct ref_array_item *ref)
> +{
> +       if (ref->kind & FILTER_REFS_DETACHED_HEAD)
> +               return get_head_description();
> +       return show_ref(&atom->u.refname, ref->refname);
> +}
> +
>  /*
>   * Parse the object referred by ref, and grab needed value.
>   */
> @@ -1181,7 +1211,6 @@ static void populate_value(struct ref_array_item *ref)
>                 struct atom_value *v = &ref->value[i];
>                 int deref = 0;
>                 const char *refname;
> -               const char *formatp;
>                 struct branch *branch = NULL;
>
>                 v->handler = append_atom;
> @@ -1192,12 +1221,10 @@ static void populate_value(struct ref_array_item *ref)
>                         name++;
>                 }
>
> -               if (starts_with(name, "refname")) {
> -                       refname = ref->refname;
> -                       if (ref->kind & FILTER_REFS_DETACHED_HEAD)
> -                               refname = get_head_description();
> -               } else if (starts_with(name, "symref"))
> -                       refname = ref->symref ? ref->symref : "";
> +               if (starts_with(name, "refname"))
> +                       refname = get_refname(atom, ref);
> +               else if (starts_with(name, "symref"))
> +                       refname = get_symref(atom, ref);
>                 else if (starts_with(name, "upstream")) {
>                         const char *branch_name;
>                         /* only local branches may have an upstream */
> @@ -1273,21 +1300,6 @@ static void populate_value(struct ref_array_item *ref)
>                 } else
>                         continue;
>
> -               formatp = strchr(name, ':');
> -               if (formatp) {
> -                       const char *arg;
> -
> -                       formatp++;
> -                       if (!strcmp(formatp, "short"))
> -                               refname = shorten_unambiguous_ref(refname,
> -                                                     warn_ambiguous_refs);
> -                       else if (skip_prefix(formatp, "strip=", &arg))
> -                               refname = strip_ref_components(refname, arg);
> -                       else
> -                               die(_("unknown %.*s format %s"),
> -                                   (int)(formatp - name), name, formatp);
> -               }
> -
>                 if (!deref)
>                         v->s = refname;
>                 else
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index b06ea1c..3d28234 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -589,4 +589,13 @@ test_expect_success 'Verify usage of %(symref:short) atom' '
>         test_cmp expected actual
>  '
>
> +cat >expected <<EOF
> +master
> +EOF
> +
> +test_expect_success 'Verify usage of %(symref:strip) atom' '
> +       git for-each-ref --format="%(symref:strip=2)" refs/heads/sym > actual &&
> +       test_cmp expected actual
> +'
> +
>  test_done
> --
> 2.10.2
>
Previous: Karthik NayakNext: Karthik Nayak
Message 46 of 94 in “port branch.c to use ref-filter's printing options”
  1. 00/17 port branch.c to use ref-filter's printing optionsKarthik Nayak, Nov 8, 2016
  2. 01/17 ref-filter: implement %(if), %(then), and %(else) atomsKarthik Nayak, Nov 8, 2016
  3. Jacob KellerNov 8, 2016
  4. Karthik NayakNov 10, 2016
  5. Junio C HamanoNov 10, 2016
  6. Karthik NayakNov 11, 2016
  7. Junio C HamanoNov 10, 2016
  8. Karthik NayakNov 11, 2016
  9. 02/17 ref-filter: include reference to 'used_atom' within 'atom_value'Karthik Nayak, Nov 8, 2016
  10. Jacob KellerNov 8, 2016
  11. Karthik NayakNov 10, 2016
  12. 03/17 ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)Karthik Nayak, Nov 8, 2016
  13. Jacob KellerNov 8, 2016
  14. Karthik NayakNov 10, 2016
  15. Jacob KellerNov 11, 2016
  16. Junio C HamanoNov 10, 2016
  17. Jacob KellerNov 11, 2016
  18. Karthik NayakNov 12, 2016
  19. Jakub NarębskiNov 18, 2016
  20. Karthik NayakNov 20, 2016
  21. 04/17 ref-filter: modify "%(objectname:short)" to take lengthKarthik Nayak, Nov 8, 2016
  22. Jacob KellerNov 8, 2016
  23. Karthik NayakNov 10, 2016
  24. Jacob KellerNov 11, 2016
  25. Karthik NayakNov 12, 2016
  26. Junio C HamanoNov 10, 2016
  27. 05/17 ref-filter: move get_head_description() from branch.cKarthik Nayak, Nov 8, 2016
  28. Jacob KellerNov 8, 2016
  29. Karthik NayakNov 10, 2016
  30. 06/17 ref-filter: introduce format_ref_array_item()Karthik Nayak, Nov 8, 2016
  31. Jacob KellerNov 8, 2016
  32. 08/17 ref-filter: add support for %(upstream:track,nobracket)Karthik Nayak, Nov 8, 2016
  33. Jacob KellerNov 8, 2016
  34. Karthik NayakNov 12, 2016
  35. 07/17 ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreamsKarthik Nayak, Nov 8, 2016
  36. Jacob KellerNov 8, 2016
  37. Karthik NayakNov 12, 2016
  38. 09/17 ref-filter: make "%(symref)" atom work with the ':short' modifierKarthik Nayak, Nov 8, 2016
  39. Jacob KellerNov 8, 2016
  40. Jakub NarębskiNov 18, 2016
  41. Karthik NayakNov 20, 2016
  42. 10/17 ref-filter: introduce refname_atom_parser_internal()Karthik Nayak, Nov 8, 2016
  43. Jakub NarębskiNov 18, 2016
  44. Karthik NayakNov 20, 2016
  45. 11/17 ref-filter: introduce symref_atom_parser() and refname_atom_parser()Karthik Nayak, Nov 8, 2016
  46. Jacob KellerNov 8, 2016
  47. Karthik NayakNov 12, 2016
  48. 12/17 ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()Karthik Nayak, Nov 8, 2016
  49. Jacob KellerNov 8, 2016
  50. 14/17 ref-filter: allow porcelain to translate messages in the outputKarthik Nayak, Nov 8, 2016
  51. Jacob KellerNov 9, 2016
  52. Jakub NarębskiNov 18, 2016
  53. Karthik NayakNov 20, 2016
  54. Matthieu MoyNov 21, 2016
  55. Karthik NayakNov 22, 2016
  56. 13/17 ref-filter: add `:dir` and `:base` options for ref printing atomsKarthik Nayak, Nov 8, 2016
  57. Jacob KellerNov 8, 2016
  58. Karthik NayakNov 13, 2016
  59. Junio C HamanoNov 14, 2016
  60. Karthik NayakNov 14, 2016
  61. Junio C HamanoNov 14, 2016
  62. Karthik NayakNov 15, 2016
  63. Jacob KellerNov 15, 2016
  64. Jacob KellerNov 15, 2016
  65. Junio C HamanoNov 15, 2016
  66. Jacob KellerNov 15, 2016
  67. Karthik NayakNov 16, 2016
  68. Junio C HamanoNov 17, 2016
  69. Karthik NayakNov 18, 2016
  70. Jacob KellerNov 18, 2016
  71. Junio C HamanoNov 18, 2016
  72. Jakub NarębskiNov 18, 2016
  73. Karthik NayakNov 20, 2016
  74. Karthik NayakNov 20, 2016
  75. Junio C HamanoNov 20, 2016
  76. Jakub NarębskiNov 20, 2016
  77. Karthik NayakNov 22, 2016
  78. 16/17 branch: use ref-filter printing APIsKarthik Nayak, Nov 8, 2016
  79. Jacob KellerNov 9, 2016
  80. Karthik NayakNov 14, 2016
  81. Jacob KellerNov 15, 2016
  82. Junio C HamanoNov 17, 2016
  83. Junio C HamanoNov 17, 2016
  84. Karthik NayakNov 22, 2016
  85. 15/17 branch, tag: use porcelain outputKarthik Nayak, Nov 8, 2016
  86. Jacob KellerNov 9, 2016
  87. 17/17 branch: implement '--format' optionKarthik Nayak, Nov 8, 2016
  88. Jacob KellerNov 9, 2016
  89. Karthik NayakNov 14, 2016
  90. Junio C HamanoNov 15, 2016
  91. Re* [PATCH v7 00/17] port branch.c to use ref-filter's printing optionsJunio C Hamano, Nov 15, 2016
  92. Karthik NayakNov 16, 2016
  93. Junio C HamanoNov 18, 2016
  94. Karthik NayakNov 20, 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.