"ZheNing Hu via GitGitGadget" [off-list ref] writes:
From: ZheNing Hu <redacted>
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.
quoted hunk
- [--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?
Junio C Hamano [off-list ref] 于2021年6月7日周一 下午1:52写道：
"ZheNing Hu via GitGitGadget" [off-list ref] writes:
quoted
From: ZheNing Hu <redacted>
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)").
First of all, although %(rest) is meaningless in ordinary circumstances,
ref-filter must learn %(rest), it is impossible for us to leave the parsing
of %(rest) in cat-file.c alone.
Then, `--rest` is a strategy that make %(rest) can use in `git for-each-ref`
or `git branch -l`. As you said, it is just a boring placeholder used for string
replacement. We can make it output only empty content, If we really don’t
need `--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)'
s/[0-3] /[0-3]\t/
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.
Yeah, the <path> in the input will be treated as "rest".
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.
"parrot the same string"? I think we should use an empty string here,
"parrot the same string" more like what the "git log --format" family does.
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.
This sounds like it might help `cat-file` to reject some useless atoms
like %(refname). So something like:
$ git for-each-ref --format="%(objectname) %(objectsize)"
--refject-atoms="%(objectsize) %(objectname)"
will fail.
"git for-each-ref" family use hardcoded to reject %(rest).
I can try to achieve this function.
quoted
- [--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?
Will output an empty string.
Hope we can reach an agreement:
delete `--rest` and add `--reject-atoms`. ;-)
Thanks.
--
ZheNing Hu
ZheNing Hu [off-list ref] 于2021年6月7日周一 下午9:02写道：
Hope we can reach an agreement:
delete `--rest` and add `--reject-atoms`. ;-)
I forget one thing: %(raw:textconv) and %(raw:filters) can use
the value of "--rest" as their <path>. But now if we want delete --rest,
they can not be used for "for-each-ref" family, Git will die with
"missing path for 'xxx'".
Thanks.
--
ZheNing Hu
ZheNing Hu [off-list ref] 于2021年6月7日周一 下午9:18写道：
ZheNing Hu [off-list ref] 于2021年6月7日周一 下午9:02写道：
quoted
Hope we can reach an agreement:
delete `--rest` and add `--reject-atoms`. ;-)
I forget one thing: %(raw:textconv) and %(raw:filters) can use
the value of "--rest" as their <path>. But now if we want delete --rest,
they can not be used for "for-each-ref" family, Git will die with
"missing path for 'xxx'".
If we actually delete "--rest", we will have no way to test %(raw:textconv)
and %(raw:filters)... So now I think we can keep --rest (or use
another name --path)
and let "git for-each-ref" family reject %(rest) by default.
Thanks.
--
ZheNing Hu
ZheNing Hu [off-list ref] writes:
First of all, although %(rest) is meaningless in ordinary circumstances,
ref-filter must learn %(rest), it is impossible for us to leave the parsing
of %(rest) in cat-file.c alone.
Oh, there is no question about that.
Then, `--rest` is a strategy that make %(rest) can use in `git for-each-ref`
or `git branch -l`.
If there is no need to expose %(rest) to the users who write
--format for these two commands, it would be much better to detect
attempted use of %(rest) and error out.
This sounds like it might help `cat-file` to reject some useless atoms
like %(refname).
Yes.
So something like:
$ git for-each-ref --format="%(objectname) %(objectsize)"
--refject-atoms="%(objectsize) %(objectname)"
will fail.
I don't understand. Why do you even need to add --reject? Why
would any user would want to use it with for-each-ref?
Without any end-user input, %(rest) for for-each-ref would not make
sense, and %(refname) for cat-file --batch would not make sense, I
would imagine, so there is no need to be able to tell --reject=rest
to for-each-ref. It is not like giving --no-reject=rest to for-each-ref
and make it interpolate to an empty string is a useful feature anyway,
so I do not see a need for such an option.
Junio C Hamano [off-list ref] 于2021年6月8日周二 下午2:50写道：
ZheNing Hu [off-list ref] writes:
quoted
First of all, although %(rest) is meaningless in ordinary circumstances,
ref-filter must learn %(rest), it is impossible for us to leave the parsing
of %(rest) in cat-file.c alone.
Oh, there is no question about that.
OK.
I don't understand. Why do you even need to add --reject? Why
would any user would want to use it with for-each-ref?
Without any end-user input, %(rest) for for-each-ref would not make
sense, and %(refname) for cat-file --batch would not make sense, I
would imagine, so there is no need to be able to tell --reject=rest
to for-each-ref. It is not like giving --no-reject=rest to for-each-ref
and make it interpolate to an empty string is a useful feature anyway,
so I do not see a need for such an option.
Yeah... I might have thought it complicated before. We can reject %(rest) in
verify_ref_format() easily. It's like we refused --<lang> before. :)
Thanks.
--
ZheNing Hu