Re: [PATCH v2 2/2] ref-filter: add support for %(contents:size)
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Jul 7, 2020, 08:40 UTC
- Message-ID
- <CAP8UFD1JiBOhpXeADObFBgoGm292dxQ933TENrZNPnmSv+SbUg@mail.gmail.com>
- In-Reply-To
- <xmqq5zb09v4e.fsf@gitster.c.googlers.com>
On Mon, Jul 6, 2020 at 11:44 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> > Christian Couder <christian.couder@gmail.com> writes: > > > It's useful and efficient to be able to get the size of the > > contents directly without having to pipe through `wc -c`. > > > > Also the result of the following: > > > > `git for-each-ref --format='%(contents)' | wc -c` > > > > is off by one as `git for-each-ref` appends a newline character > > after the contents, which can be seen by comparing its ouput > > with the output from `git cat-file`. > > So that's off by number of refs that are shown?
Yeah, true. I should have added a ref as I mean something like the following is off by one:
`git for-each-ref --format='%(contents)' refs/heads/my-branch | wc -c`
I have changed it to the above in my current version.
Show 5 quoted lines
> > +contents:size:: > > + The size in bytes of the complete message. > > + > > Complete as opposed to what?
In the existing documentation there is already "The complete message of a commit or tag object is `contents`. "
So yeah I could add another preparatory patch to change the above to something like:
"The complete message (subject, body, trailers and signature) of a commit or tag object is `contents`. "
> What happens when the object referred to by the ref is not a commit > or a tag?
Right now %(contents) shows nothing (a blank line actually) when the ref points to something other than a commit or a tag, and %(contents:size) does the same:
``` $ git update-ref refs/mytrees/first HEAD^{tree} $ git for-each-ref --format='%(contents)' refs/mytrees/first
$ git for-each-ref --format='%(contents:size)' refs/mytrees/first
```
I am not sure if it's worth updating the existing documentation or just the commit message of this patch. For now I have done the latter in my current version.
Show 6 quoted lines
> I am fine if it just is silently ignored (which is consistent with > already existing behaviour of other requests that do not make sense > for the given type) if the thing is a blob or a tree, but we'd need > to cover the case with a test or two. It seems you only expect this > with a tag object and do not have any test that checks for other > types of objects?
My patch already adds 1 test with a commit:
--- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -125,6 +125,7 @@ test_atom head contents:body '' test_atom head contents:signature '' test_atom head contents 'Initial ' +test_atom head contents:size '8' test_atom head HEAD '*' There is only one test with a commit, because that's already the case for %(contents) too. I am ok with adding another preparatory patch to the series that would add a few more test cases with commits, trees and blobs though.