From: Junio C Hamano <gitster@pobox.com>
To: ZheNing Hu <adlternative@gmail.com>
Cc: ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com>,
Git List <git@vger.kernel.org>,
Christian Couder <christian.couder@gmail.com>,
Hariom Verma <hariom18599@gmail.com>
Subject: Re: [PATCH 4/6] [GSOC] ref-filter: add %(rest) atom and --rest option
Date: Wed, 09 Jun 2021 16:00:25 +0900 [thread overview]
Message-ID: <xmqq4ke7jzee.fsf@gitster.g> (raw)
In-Reply-To: xmqqk0n4q1t6.fsf@gitster.g
Junio C Hamano <gitster@pobox.com> 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.
next prev parent reply other threads:[~2021-06-09 7:00 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-05 9:13 [PATCH 0/6] [GSOC][RFC] ref-filter: add %(raw:textconv) and %(raw:filters) ZheNing Hu via GitGitGadget
2021-06-05 9:13 ` [PATCH 1/6] [GSOC] ref-filter: add obj-type check in grab contents ZheNing Hu via GitGitGadget
2021-06-05 9:13 ` [PATCH 2/6] [GSOC] ref-filter: add %(raw) atom ZheNing Hu via GitGitGadget
2021-06-08 5:07 ` Junio C Hamano
2021-06-08 6:10 ` ZheNing Hu
2021-06-05 9:13 ` [PATCH 3/6] [GSOC] ref-filter: use non-const ref_format in *_atom_parser() ZheNing Hu via GitGitGadget
2021-06-05 9:13 ` [PATCH 4/6] [GSOC] ref-filter: add %(rest) atom and --rest option ZheNing Hu via GitGitGadget
2021-06-05 15:20 ` Hariom verma
2021-06-06 4:58 ` ZheNing Hu
2021-06-07 5:52 ` Junio C Hamano
2021-06-07 13:02 ` ZheNing Hu
2021-06-07 13:18 ` ZheNing Hu
2021-06-08 6:16 ` ZheNing Hu
2021-06-08 6:59 ` Junio C Hamano
2021-06-08 12:39 ` ZheNing Hu
2021-06-09 7:00 ` Junio C Hamano [this message]
2021-06-09 12:47 ` ZheNing Hu
2021-06-08 6:50 ` Junio C Hamano
2021-06-08 12:32 ` ZheNing Hu
2021-06-05 9:13 ` [PATCH 5/6] [GSOC] ref-filter: teach grab_sub_body_contents() return value and err ZheNing Hu via GitGitGadget
2021-06-05 9:13 ` [PATCH 6/6] [GSOC] ref-filter: add %(raw:textconv) and %(raw:filters) ZheNing Hu via GitGitGadget
2021-06-05 10:29 ` [PATCH 0/6] [GSOC][RFC] " Bagas Sanjaya
2021-06-05 13:19 ` ZheNing Hu
2021-06-07 5:55 ` Junio C Hamano
2021-06-07 13:06 ` ZheNing Hu
2021-06-08 6:42 ` Junio C Hamano
2021-06-08 12:52 ` ZheNing Hu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq4ke7jzee.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=adlternative@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=hariom18599@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).