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

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

From
Jacob Keller <jacob.keller@gmail.com>
Date
Dec 8, 2016, 00:01 UTC
Message-ID
<CA+P7+xquordVY19dypqNcAuQqoRbFmHhzb0w+HXCaJmm_Ex7zQ@mail.gmail.com>
In-Reply-To
<20161207153627.1468-1-Karthik.188@gmail.com>
On Wed, Dec 7, 2016 at 7:36 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 89 quoted lines
> This is part of unification of the commands 'git tag -l, git branch -l
> and git for-each-ref'. This ports over branch.c to use ref-filter's
> printing options.
>
> Initially posted here: $(gmane/279226). It was decided that this series
> would follow up after refactoring ref-filter parsing mechanism, which
> is now merged into master (9606218b32344c5c756f7c29349d3845ef60b80c).
>
> v1 can be found here: $(gmane/288342)
> v2 can be found here: $(gmane/288863)
> v3 can be found here: $(gmane/290299)
> v4 can be found here: $(gmane/291106)
> v5b can be found here: $(gmane/292467)
> v6 can be found here: http://marc.info/?l=git&m=146330914118766&w=2
> v7 can be found here: http://marc.info/?l=git&m=147863593317362&w=2
>
> Changes in this version:
>
> 1. use an enum for holding the comparision type in
> %(if:[equals/notequals=...]) options.
> 2. rename the 'strip' option to 'lstrip' and introduce an 'rstrip'
> option. Also modify them to take negative values. This drops the
> ':dri' and ':base' options.
> 3. Drop unecessary code.
> 4. Cleanup code and fix spacing.
> 5. Add more comments wherever required.
> 6. Add quote_literal_for_format(const char *s) for safer string
> insertions in branch.c:build_format().
>
> Thanks to Jacob, Jackub, Junio and Matthieu for their inputs on the
> previous version.
>
> Interdiff below.
>
> Karthik Nayak (19):
>   ref-filter: implement %(if), %(then), and %(else) atoms
>   ref-filter: include reference to 'used_atom' within 'atom_value'
>   ref-filter: implement %(if:equals=<string>) and
>     %(if:notequals=<string>)
>   ref-filter: modify "%(objectname:short)" to take length
>   ref-filter: move get_head_description() from branch.c
>   ref-filter: introduce format_ref_array_item()
>   ref-filter: make %(upstream:track) prints "[gone]" for invalid
>     upstreams
>   ref-filter: add support for %(upstream:track,nobracket)
>   ref-filter: make "%(symref)" atom work with the ':short' modifier
>   ref-filter: introduce refname_atom_parser_internal()
>   ref-filter: introduce refname_atom_parser()
>   ref-filter: make remote_ref_atom_parser() use
>     refname_atom_parser_internal()
>   ref-filter: rename the 'strip' option to 'lstrip'
>   ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
>   ref-filter: add an 'rstrip=<N>' option to atoms which deal with
>     refnames
>   ref-filter: allow porcelain to translate messages in the output
>   branch, tag: use porcelain output
>   branch: use ref-filter printing APIs
>   branch: implement '--format' option
>
>  Documentation/git-branch.txt       |   7 +-
>  Documentation/git-for-each-ref.txt |  86 +++++--
>  builtin/branch.c                   | 290 +++++++---------------
>  builtin/tag.c                      |   6 +-
>  ref-filter.c                       | 488 +++++++++++++++++++++++++++++++------
>  ref-filter.h                       |   7 +
>  t/t3203-branch-output.sh           |  16 +-
>  t/t6040-tracking-info.sh           |   2 +-
>  t/t6300-for-each-ref.sh            |  88 ++++++-
>  t/t6302-for-each-ref-filter.sh     |  94 +++++++
>  10 files changed, 784 insertions(+), 300 deletions(-)
>
> Interdiff:
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index f4ad297..c72baeb 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -92,13 +92,14 @@ refname::
>         The name of the ref (the part after $GIT_DIR/).
>         For a non-ambiguous short name of the ref append `:short`.
>         The option core.warnAmbiguousRefs is used to select the strict
> -       abbreviation mode. If `strip=<N>` is appended, strips `<N>`
> -       slash-separated path components from the front of the refname
> -       (e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
> -       `<N>` must be a positive integer.  If a displayed ref has fewer
> -       components than `<N>`, the command aborts with an error. For the base
> -       directory of the ref (i.e. foo in refs/foo/bar/boz) append
> -       `:base`. For the entire directory path append `:dir`.
> +       abbreviation mode. If `lstrip=<N>` or `rstrip=<N>` option can

Grammar here, drop the If before `lstrip since you're referring to multiples and you say "x can be appended to y" rather than "if x is added, do y"

Show 8 quoted lines
> +       be appended to strip `<N>` slash-separated path components
> +       from or end of the refname respectively (e.g.,
> +       `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo` and
> +       `%(refname:rstrip=2)` turns `refs/tags/foo` into `refs`).  if
> +       `<N>` is a negative number, then only `<N>` path components
> +       are left behind.  If a displayed ref has fewer components than
> +       `<N>`, the command aborts with an error.
>

Would it make more sense to not die and instead just return the empty string? On the one hand, if we die() it's obvious that you tried to strip too many components. But on the other hand, it's also somewhat annoying to have the whole command fail because we happen upon a single ref that has fewer components?

So, for positive numbers, we simply strip what we can, which may result in the empty string, and for negative numbers, we keep up to what we said, while potentially keeping the entire string. I feel that's a better alternative than a die() in the middle of a ref filter..

What are other people's thoughts on this?

Thanks, Jake

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