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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 7, 2021, 05:52 UTC
Message-ID: <xmqq7dj6w7a6.fsf@gitster.g>
In-Reply-To: <ccdd18ad508824aa206a02c479229d0ede69522d.1622884415.git.gitgitgadget@gmail.com>

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 7 quoted lines

> From: ZheNing Hu <adlternative@gmail.com>
>
> In order to let "cat-file --batch=%(rest)" use the ref-filter
> interface, add %(rest) atom for ref-filter and --rest option
> for "git for-each-ref", "git branch", "git tag" and "git verify-tag".
> `--rest` specify a string to replace %(rest) placeholders of
> the --format option.

I cannot think of a sane reason why we need to allow "%(rest)" in anithing but "cat-file --batch", where a natural source of %(rest) exists in its input stream (i.e. each input record begins with an object name to be processed, and the rest of the record can become "%(rest)").

The "cat-file --batch" thing is much more understandable. You could for example:

    git ls-files -s |
    sed -e 's/^[0-7]* \([0-9a-f]*\) [0-3]	/\1 /' |
    git cat-file --batch='%(objectname) %(objecttype) %(rest)'

to massage output from "ls-files -s" like this

    100644 c2f5fe385af1bbc161f6c010bdcf0048ab6671ed 0	.cirrus.yml
    100644 c592dda681fecfaa6bf64fb3f539eafaf4123ed8 0	.clang-format
    100644 f9d819623d832113014dd5d5366e8ee44ac9666a 0	.editorconfig
    ...

into recods of "<objectname> <path>", and each output record will replay the <path> part from each corresponding input record.

Unless for-each-ref family of commands read the list of refs that it shows from their standard input (they do not, and I do not think it makes any sense to teach them to), there is no place to feed the "rest" information that is associated with each output record. The only thing the commands taught about %(rest) by this patch can do is to parrot the same string into each and every output record. I am not seeing what this new feature is attempting to give us.

If anything, I would imagine that it would be a very useful addition to teach the ref-filter machinery an ability to optionally error out depending on the caller when the caller attempts to use certain placeholder. Then, we can reject "git branch --sort=rest" sensibly, instead of accepting "git branch --sort=rest --rest=constant", which is not technically wrong per-se, but smells like a total nonsense from practical usefulness's point of view.

Show 12 quoted lines

> -	[--list] [<pattern>...]
> +	[--list] [<pattern>...] [--rest=<rest>]
>  'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
>  'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
>  'git branch' --unset-upstream [<branchname>]
> @@ -298,6 +298,10 @@ start-point is either a local or remote-tracking branch.
>  	and the object it points at.  The format is the same as
>  	that of linkgit:git-for-each-ref[1].
>  
> +--rest=<rest>::
> +	If given, the `%(rest)` placeholders in the `--format` option
> +	will be replaced.

If not given, what happens?