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

Re: [PATCH v2 09/11] ref-filter: introduce remote_ref_atom_parser()

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Dec 17, 2015, 09:22 UTC
Message-ID
<CAPig+cRG_HTWL=VNqOxDnv_M0NbYZSReCHSsGe1CJiFQS8h-XQ@mail.gmail.com>
In-Reply-To
<1450279802-29414-10-git-send-email-Karthik.188@gmail.com>
On Wed, Dec 16, 2015 at 10:30 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 14 quoted lines
> Introduce remote_ref_atom_parser() which will parse the '%(upstream)'
> and '%(push)' atoms and store information into the 'used_atom'
> structure based on the modifiers used along with the corresponding
> atom.
>
> Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -37,6 +37,8 @@ static struct used_atom {
>         union {
>                 const char *color;
>                 struct align align;
> +               enum { RR_SHORTEN, RR_TRACK, RR_TRACKSHORT, RR_NORMAL }
Nit: I'd have expected to see the normal/plain case first rather than
last (but not itself worth a re-roll).
Show 13 quoted lines
> +                       remote_ref;
>         } u;
>  } *used_atom;
>  static int used_atom_cnt, need_tagged, need_symref;
> @@ -69,6 +71,25 @@ static void color_atom_parser(struct used_atom *atom)
> +static void remote_ref_atom_parser(struct used_atom *atom)
> +{
> +       const char *buf;
> +
> +       buf = strchr(atom->str, ':');
> +       atom->u.remote_ref = RR_NORMAL;
> +       if (!buf)
> +               return;

This code is not as clear as it could be due to the way the 'buf' assignment and check for NULL are split apart. It can be made clearer either by doing this:

    atom->u.remote_ref = RR_NORMAL;
    buf = strchr(...);
    if (!buf)
        return;
or (even better) this:
    buf = strchr(...);
    if (!buf) {
        atom->u.remote_ref = RR_NORMAL;
        return;
    }
Show 22 quoted lines
> +       buf++;
> +       if (!strcmp(buf, "short"))
> +               atom->u.remote_ref = RR_SHORTEN;
> +       else if (!strcmp(buf, "track"))
> +               atom->u.remote_ref = RR_TRACK;
> +       else if (!strcmp(buf, "trackshort"))
> +               atom->u.remote_ref = RR_TRACKSHORT;
> +       else
> +               die(_("unrecognized format: %%(%s)"), atom->str);
> +}
> +
> @@ -835,6 +856,42 @@ static inline char *copy_advance(char *dst, const char *src)
> +static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
> +                                   struct branch *branch, const char **s)
> +{
> +       int num_ours, num_theirs;
> +       if (atom->u.remote_ref == RR_SHORTEN)
> +               *s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
> +       else if (atom->u.remote_ref == RR_TRACK) {
> +               if (stat_tracking_info(branch, &num_ours,
> +                                      &num_theirs, NULL))
> +                       return;

The RR_TRACKSHORT branch below has a blank line following the 'return', but this branch lacks it, which is inconsistent.

Show 12 quoted lines
> +               if (!num_ours && !num_theirs)
> +                       *s = "";
> +               else if (!num_ours)
> +                       *s = xstrfmt("[behind %d]", num_theirs);
> +               else if (!num_theirs)
> +                       *s = xstrfmt("[ahead %d]", num_ours);
> +               else
> +                       *s = xstrfmt("[ahead %d, behind %d]",
> +                                    num_ours, num_theirs);
> +       } else if (atom->u.remote_ref == RR_TRACKSHORT) {
> +               if (stat_tracking_info(branch, &num_ours,
> +                                      &num_theirs, NULL))

What happened to the assert(branch) which was in the original code from which this was derived (below)? Is it no longer needed?

Show 88 quoted lines
> +                       return;
> +
> +               if (!num_ours && !num_theirs)
> +                       *s = "=";
> +               else if (!num_ours)
> +                       *s = "<";
> +               else if (!num_theirs)
> +                       *s = ">";
> +               else
> +                       *s = "<>";
> +       } else if (atom->u.remote_ref == RR_NORMAL)
> +               *s = refname;
> +}
> +
>  /*
>   * Parse the object referred by ref, and grab needed value.
>   */
> @@ -889,6 +946,8 @@ static void populate_value(struct ref_array_item *ref)
>                         refname = branch_get_upstream(branch, NULL);
>                         if (!refname)
>                                 continue;
> +                       fill_remote_ref_details(atom, refname, branch, &v->s);
> +                       continue;
>                 } else if (starts_with(name, "push")) {
>                         const char *branch_name;
>                         if (!skip_prefix(ref->refname, "refs/heads/",
> @@ -899,6 +958,8 @@ static void populate_value(struct ref_array_item *ref)
>                         refname = branch_get_push(branch, NULL);
>                         if (!refname)
>                                 continue;
> +                       fill_remote_ref_details(atom, refname, branch, &v->s);
> +                       continue;
>                 } else if (starts_with(name, "color:")) {
>                         char color[COLOR_MAXLEN] = "";
>
> @@ -944,49 +1005,11 @@ static void populate_value(struct ref_array_item *ref)
>
>                 formatp = strchr(name, ':');
>                 if (formatp) {
> -                       int num_ours, num_theirs;
> -
>                         formatp++;
>                         if (!strcmp(formatp, "short"))
>                                 refname = shorten_unambiguous_ref(refname,
>                                                       warn_ambiguous_refs);
> -                       else if (!strcmp(formatp, "track") &&
> -                                (starts_with(name, "upstream") ||
> -                                 starts_with(name, "push"))) {
> -
> -                               if (stat_tracking_info(branch, &num_ours,
> -                                                      &num_theirs, NULL))
> -                                       continue;
> -
> -                               if (!num_ours && !num_theirs)
> -                                       v->s = "";
> -                               else if (!num_ours)
> -                                       v->s = xstrfmt("[behind %d]", num_theirs);
> -                               else if (!num_theirs)
> -                                       v->s = xstrfmt("[ahead %d]", num_ours);
> -                               else
> -                                       v->s = xstrfmt("[ahead %d, behind %d]",
> -                                                      num_ours, num_theirs);
> -                               continue;
> -                       } else if (!strcmp(formatp, "trackshort") &&
> -                                  (starts_with(name, "upstream") ||
> -                                   starts_with(name, "push"))) {
> -                               assert(branch);
> -
> -                               if (stat_tracking_info(branch, &num_ours,
> -                                                       &num_theirs, NULL))
> -                                       continue;
> -
> -                               if (!num_ours && !num_theirs)
> -                                       v->s = "=";
> -                               else if (!num_ours)
> -                                       v->s = "<";
> -                               else if (!num_theirs)
> -                                       v->s = ">";
> -                               else
> -                                       v->s = "<>";
> -                               continue;
> -                       } else
> +                       else
>                                 die("unknown %.*s format %s",
>                                     (int)(formatp - name), name, formatp);
>                 }
> --
> 2.6.4
Previous: Karthik NayakNext: Karthik Nayak
Message 25 of 34 in “ref-filter: use parsing functions”
  1. 00/11 ref-filter: use parsing functionsKarthik Nayak, Dec 16, 2015
  2. 01/11 strbuf: introduce strbuf_split_str_without_term()Karthik Nayak, Dec 16, 2015
  3. Eric SunshineDec 16, 2015
  4. Karthik NayakDec 17, 2015
  5. 02/11 ref-filter: use strbuf_split_str_omit_term()Karthik Nayak, Dec 16, 2015
  6. 03/11 ref-filter: introduce struct used_atomKarthik Nayak, Dec 16, 2015
  7. Eric SunshineDec 16, 2015
  8. Karthik NayakDec 19, 2015
  9. 04/11 ref-fitler: bump match_atom() name to the topKarthik Nayak, Dec 16, 2015
  10. 05/11 ref-filter: skip deref specifier in match_atom_name()Karthik Nayak, Dec 16, 2015
  11. Eric SunshineDec 16, 2015
  12. Karthik NayakDec 19, 2015
  13. 06/11 ref-filter: introduce color_atom_parser()Karthik Nayak, Dec 16, 2015
  14. Eric SunshineDec 16, 2015
  15. Karthik NayakDec 19, 2015
  16. 07/11 ref-filter: introduce align_atom_parser()Karthik Nayak, Dec 16, 2015
  17. Eric SunshineDec 16, 2015
  18. Karthik NayakDec 19, 2015
  19. Karthik NayakDec 19, 2015
  20. Eric SunshineDec 18, 2015
  21. 08/11 ref-filter: introduce prefixes for the align atomKarthik Nayak, Dec 16, 2015
  22. Eric SunshineDec 17, 2015
  23. Karthik NayakDec 31, 2015
  24. 09/11 ref-filter: introduce remote_ref_atom_parser()Karthik Nayak, Dec 16, 2015
  25. Eric SunshineDec 17, 2015
  26. Karthik NayakDec 24, 2015
  27. 10/11 ref-filter: introduce contents_atom_parser()Karthik Nayak, Dec 16, 2015
  28. Eric SunshineDec 17, 2015
  29. Karthik NayakDec 24, 2015
  30. 11/11 ref-filter: introduce objectname_atom_parser()Karthik Nayak, Dec 16, 2015
  31. Eric SunshineDec 18, 2015
  32. Karthik NayakDec 25, 2015
  33. Eric SunshineDec 25, 2015
  34. Karthik NayakDec 25, 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.