Re: [PATCH 4/6] [GSOC] ref-filter: add %(rest) atom and --rest option

From: Hariom verma <hariom18599@gmail.com>
To: ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com>
Cc: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
	Christian Couder <christian.couder@gmail.com>,
	ZheNing Hu <adlternative@gmail.com>
Subject: Re: [PATCH 4/6] [GSOC] ref-filter: add %(rest) atom and --rest option
Date: Sat, 5 Jun 2021 20:50:37 +0530	[thread overview]
Message-ID: <CA+CkUQ9f8kN=S8dU_zt=-uG1pcK8cE9CuhJdqR9oMwcguZ9FLg@mail.gmail.com> (raw)
In-Reply-To: <ccdd18ad508824aa206a02c479229d0ede69522d.1622884415.git.gitgitgadget@gmail.com>

Hi,

On Sat, Jun 5, 2021 at 2:43 PM ZheNing Hu via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: ZheNing Hu <adlternative@gmail.com>
>
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -670,6 +674,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                         N_("print only branches of the object"), parse_opt_object_name),
>                 OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
>                 OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
> +               OPT_STRING(0, "rest", &format.rest, N_("rest"), N_("specify %(rest) contents")),
>                 OPT_END(),
>         };
>

Although it's not related to this patch. But I just noticed an unusual
extra space(s) before the first argument of `OPT_STRING()`. (above the
line you added)

> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -37,6 +37,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>
>                 OPT_GROUP(""),
>                 OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
> +               OPT_STRING(  0 , "rest", &format.rest, N_("rest"), N_("specify %(rest) contents")),
>                 OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
>                 OPT__COLOR(&format.use_color, N_("respect format colors")),
>                 OPT_REF_SORT(sorting_tail),

Here too in `OPT_INTEGER()` and `OPT_INTEGER()`.

Also, I don't think these extra space(s) are intended. So you don't
need to imitate them.

> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -481,6 +486,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>                 OPT_STRING(  0 , "format", &format.format, N_("format"),
>                            N_("format to use for the output")),
>                 OPT__COLOR(&format.use_color, N_("respect format colors")),
> +               OPT_STRING(0, "rest", &format.rest, N_("rest"), N_("specify %(rest) contents")),
>                 OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
>                 OPT_END()
>         };

Here too in the first line.

Thanks,
Hariom