ZheNing Hu [off-list ref] writes:
ZheNing Hu [off-list ref] 于2021年6月7日周一 下午9:18写道：
quoted
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.
I didn't read beyond the %(rest) thing, but do we even need
%(raw:textconv) to begin with? It is totally useless in the context
of for-each-ref because textconv by its nature is tied to attributes
that by definition needs a blob that is sitting at a path, but the
objects for-each-ref and friends visit are mostly commits and tags,
and even for refs that point at a blob, there isn't any "path"
information to pull attribute for.
Is that what you want to add to give "cat-file --batch"? Even in
the context of "cat-file --batch", you can throw an object name for
a blob to the command, but there is no path for the blob (a blob can
appear at different places in different trees---think "rename), so I
am not sure what benefit you are trying to derive from it.
Thanks.
Junio C Hamano [off-list ref] 于2021年6月8日周二 下午2:59写道：
quoted
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.
I didn't read beyond the %(rest) thing, but do we even need
%(raw:textconv) to begin with? It is totally useless in the context
of for-each-ref because textconv by its nature is tied to attributes
that by definition needs a blob that is sitting at a path, but the
objects for-each-ref and friends visit are mostly commits and tags,
and even for refs that point at a blob, there isn't any "path"
information to pull attribute for.
After thinking about your words, now I think maybe we can leave
%(raw:textconv) and %(raw:filter) after cat-file --batch start using
ref-filter logic, so that we can provide them with suitable tests,
and we don't need `--rest` anymore.
Is that what you want to add to give "cat-file --batch"? Even in
the context of "cat-file --batch", you can throw an object name for
a blob to the command, but there is no path for the blob (a blob can
appear at different places in different trees---think "rename), so I
am not sure what benefit you are trying to derive from it.
So I will remove the last two commits.
Thanks.
Thanks.
--
ZheNing Hu
Junio C Hamano [off-list ref] writes:
Is that what you want to add to give "cat-file --batch"? Even in
the context of "cat-file --batch", you can throw an object name for
a blob to the command, but there is no path for the blob (a blob can
appear at different places in different trees---think "rename), so I
am not sure what benefit you are trying to derive from it.
I think I kind-of see what is going on here. There is
git cat-file blob --textconv --path="$path" "$blob_object_name"
that allows a blob to be fed to the command, pretend as if it
appears at $path in a tree object and grab attribute for it, and
show the blob contents converted using the textconv filter. If we
were to mimic it by extending the format based substitutions, a
design consistent with the behaviour is to teach --format=%(raw)
to show the contents after applying the textconv filter instead of
the raw blob contents.
And there is a corresponding
git cat-file --batch --textconv
The "--path=$path" parameter is omitted when using --batch, as each
object would sit at different path in the tree (so the input stream
would be given as a run of "<blob> <path>" to give each item its own
path).
So to answer my question in the previous message, yes, this is an
attempt to support the "cat-file --textconv". So in the context of
that command, something may need to be added. But I do not think it
makes any sense to expose that to for-each-ref and friends, even if
we were to share the internal machinery (after all, sharing of the
internal machinery is a mere means to an end that is to make it
easier to give the same syntax and same behaviour to end users and
is not a goal itself; "because we use the same machinery, the users
have to tolerate that irrelevant %(atoms) are accepted by the parser"
is not making a good excuse for a sloppy implementation).
Having said all that, I somehow doubt that the "--batch=<format>"
was designed to interact sensibly with the "--textconv" option.
builtin/cat-file.c::expand_atom() does not know anything at all that
the data could be modified from the raw contents of the blob, so
--batch="%(contents) %(size)" --textconv, if existed, may show the
conveted contents with size of blob before conversion, or something
incoherent like that. And if your rewrite using the shared internal
machinery results in a more coherent behaviour, that would be
excellent. For example, we could imagine that the machinery, when
textconv (or filter) is in use, would first grab the blob contents
and run the requested conversion, and then work solely on that
conveted contents when computing what to fill with %(raw:size) and
other blob-related atoms.
Thanks.
Junio C Hamano [off-list ref] 于2021年6月9日周三 下午3:00写道：
I think I kind-of see what is going on here. There is
git cat-file blob --textconv --path="$path" "$blob_object_name"
that allows a blob to be fed to the command, pretend as if it
appears at $path in a tree object and grab attribute for it, and
show the blob contents converted using the textconv filter. If we
were to mimic it by extending the format based substitutions, a
design consistent with the behaviour is to teach --format=%(raw)
to show the contents after applying the textconv filter instead of
the raw blob contents.
Yes, this is exactly what cat-file --textconv does.
And there is a corresponding
git cat-file --batch --textconv
The "--path=$path" parameter is omitted when using --batch, as each
object would sit at different path in the tree (so the input stream
would be given as a run of "<blob> <path>" to give each item its own
path).
Just like let --batch omitted --path, --rest is meaningless for "for-ech-ref".
So to answer my question in the previous message, yes, this is an
attempt to support the "cat-file --textconv". So in the context of
that command, something may need to be added. But I do not think it
makes any sense to expose that to for-each-ref and friends, even if
we were to share the internal machinery (after all, sharing of the
internal machinery is a mere means to an end that is to make it
easier to give the same syntax and same behaviour to end users and
is not a goal itself; "because we use the same machinery, the users
have to tolerate that irrelevant %(atoms) are accepted by the parser"
is not making a good excuse for a sloppy implementation).
Because "git cat-file --batch" will only print the contents of the object once,
so when implements the function of textconv/filters in ref-filter,
we should really consider whether we should let something like
"%(raw) %(raw) %(raw) %(raw:size)" all pass the conversion of textconv/filters.
If it is my previous %(raw:textconv) or %(raw:filter), they can only print
the converted content separately, and we need:
$ git for-ecah-ref --format="%(raw:filters) %(raw:filters)
%(raw:filters) %(raw:filters:size)"
As you said it might be too complicated for the user....
Having said all that, I somehow doubt that the "--batch=<format>"
was designed to interact sensibly with the "--textconv" option.
builtin/cat-file.c::expand_atom() does not know anything at all that
the data could be modified from the raw contents of the blob, so
--batch="%(contents) %(size)" --textconv, if existed, may show the
conveted contents with size of blob before conversion, or something
incoherent like that. And if your rewrite using the shared internal
machinery results in a more coherent behaviour, that would be
excellent. For example, we could imagine that the machinery, when
textconv (or filter) is in use, would first grab the blob contents
and run the requested conversion, and then work solely on that
conveted contents when computing what to fill with %(raw:size) and
other blob-related atoms.
And with your --textconv/--filters, we only need:
$ git for-ecah-ref --format="%(raw) %(raw) %(raw) %(raw:size)" --filters
This will be more concise for users. I will try to build --filters, --textconv
for ref-filter . But as stated in the previous reply, it needs to be placed
after the transplant of cat-file --batch (Because --path is useless for
for-each-ref, and at the same time we need proper testing for
--filter/--textconv.)
Thanks, your reply is very reasonable,
--
ZheNing Hu