Re: [GSoC][RFC PATCH 1/2] builtin/refs: add list subcommand
- From
Jean-Noël Avila <jn.avila@free.fr>
- Date
- Jun 27, 2025, 16:27 UTC
- Message-ID
- <0d3fe70e-2064-4206-9673-c0589a4445f1@free.fr>
- In-Reply-To
- <20250627074934.1761897-2-meetsoni3017@gmail.com>
Hello,
I'm only focusing on the documentation part.
Le 27/06/2025 à 09:49, Meet Soni a écrit :
Show 15 quoted lines
> > --- > Documentation/git-refs.adoc | 95 +++++++++++++++++++++++++++++++ > builtin/refs.c | 110 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 205 insertions(+) > > diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc > index 4d6dc994f9..d8f81eaabd 100644 > --- a/Documentation/git-refs.adoc > +++ b/Documentation/git-refs.adoc > @@ -11,6 +11,13 @@ SYNOPSIS > [synopsis] > git refs migrate --ref-format=<format> [--no-reflog] [--dry-run] > git refs verify [--strict] [--verbose] > +git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
please use spaces around '|' in alternative options: [--shell | --perl | --python | --tcl]
> + [(--sort=<key>)...] [--format=<format>]
This "[(--sort=<key>)...]" form is new to me. It abides by the synopsis syntax. As I understand it, the user can specify a number of --sort=<key> on the command line.
From the description below, it seems that for --format, the actual syntax should be:
[--format[=<format>]]
> + [--include-root-refs] [ --stdin | <pattern>... ]
Here no spaces, after '[' or before ']': [--stdin | <pattern>...]
For syntax precedence, we have not documented anything, but the obvious meaning is "--stdin or a number of <pattern>. OK
> + [--points-at=<object>] > + [--merged[=<object>]] [--no-merged[=<object>]] > + [--contains[=<object>]] [--no-contains[=<object>]] > + [--exclude=<pattern> ...]
What are the ... meaning here ? Is it --exclude= followed by a number of <pattern>? What is the separator then? Or is it like for --sort?
Maybe use another placeholder name to differentiate from the <pattern> alternative to --stdin
Show 8 quoted lines
> > DESCRIPTION > ----------- > @@ -26,6 +33,11 @@ migrate:: > verify:: > Verify reference database consistency. > > +list::
OK, synopsis is used in the first part, but this man page has not been fully converted. Let's stick to the previous style.
Show 25 quoted lines
> + List references in the repository with support for filtering, formatting, > + and sorting. This subcommand uses the same core logic as > + linkgit:git-for-each-ref[1] and offers equivalent functionality. > + > OPTIONS > ------- > > @@ -57,6 +69,89 @@ The following options are specific to 'git refs verify': > --verbose:: > When verifying the reference database consistency, be chatty. > > +The following options are specific to 'git refs list': > + > +<pattern>...:: > + If one or more patterns are given, only refs are shown that > + match against at least one pattern, either using fnmatch(3) or > + literally, in the latter case matching completely or from the > + beginning up to a slash. > +> +--stdin:: > + If `--stdin` is supplied, then the list of patterns is read from > + standard input instead of from the argument list.> + > +--count=<count>:: > + By default the command shows all refs that match > + `<pattern>`. This option makes it stop after showing > + that many refs.
Please revert the sentences and remove the "this option makes it". The option description should state first the action of the option, then talk about default values, behavior, ...
> + > +--sort=<key>:: > + A field name to sort on. Prefix `-` to sort in
Use a verb in imperative mood as much as possible.
Show 6 quoted lines
> + descending order of the value. When unspecified, > + `refname` is used. You may use the --sort=<key> option > + multiple times, in which case the last key becomes the primary > + key. > + > +--format=<format>::
Cite the form with optional param
> + A string that interpolates `%(fieldname)` from a ref being shown and > + the object it points at. In addition, the string literal `%%` > + renders as `%` and `%xx` - where `xx` are hex digits - renders as > + the character with hex code `xx`. For example, `%00` interpolates to
the ASCII character only I guess: encoding matters here as we are usually in UTF-8.
Show 15 quoted lines
> + `\0` (NUL), `%09` to `\t` (TAB), and `%0a` to `\n` (LF). > ++ > +When unspecified, `<format>` defaults to `%(objectname) SPC %(objecttype) > +TAB %(refname)`. > + > +--color[=<when>]:: > + Respect any colors specified in the `--format` option. The > + `<when>` field must be one of `always`, `never`, or `auto` (if > + `<when>` is absent, behave as if `always` was given). > + > +--shell:: > +--perl:: > +--python:: > +--tcl:: > + If given, strings that substitute `%(fieldname)`
Personal taste: "If given" is useless here.
Show 37 quoted lines
> + placeholders are quoted as string literals suitable for > + the specified host language. This is meant to produce > + a scriptlet that can directly be `eval`ed. > + > +--points-at=<object>:: > + Only list refs which points at the given object. > + > +--merged[=<object>]:: > + Only list refs whose tips are reachable from the > + specified commit (HEAD if not specified). > + > +--no-merged[=<object>]:: > + Only list refs whose tips are not reachable from the > + specified commit (HEAD if not specified). > + > +--contains[=<object>]:: > + Only list refs which contain the specified commit (HEAD if not > + specified). > + > +--no-contains[=<object>]:: > + Only list refs which don't contain the specified commit (HEAD > + if not specified). > + > +--ignore-case:: > + Sorting and filtering refs are case insensitive. > + > +--omit-empty:: > + Do not print a newline after formatted refs where the format expands > + to the empty string. > + > +--exclude=<pattern>:: > + If one or more patterns are given, only refs which do not match > + any excluded pattern(s) are shown. Matching is done using the > + same rules as `<pattern>` above. > + > +--include-root-refs:: > + List root refs (HEAD and pseudorefs) apart from regular refs.
The description seems a bit off. Did you really mean "apart from", not "along with"?
Thanks
Jean-Noël