| Message ID | 20200702140845.24945-1-chriscool@tuxfamily.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for %(contents:size) in ref-filter | expand |
On Thu, Jul 02, 2020 at 04:08:43PM +0200, Christian Couder wrote: > This is version 2 of a very small patch series to teach ref-filter > about %(contents:size). > > Version 1 consisted on a single patch which is available here: > > https://lore.kernel.org/git/20200701132308.16691-1-chriscool@tuxfamily.org/ > > As suggested by Peff, I added a preparatory patch (1/2) to clean up > the documentation about the %(contents:XXXX) format specifiers. Thanks, I think it looks much nicer. > The other difference with V1 is that there are more tests in patch > 2/2. These new tests required a small helper function to be > introduced. I'm still not sure why %(objectsize) isn't sufficient here. Is there some use case that's served by %(contents:size) that it wouldn't work for? Or are we just trying to make it more discoverable when you're looking at the contents already? -Peff
On Tue, Jul 7, 2020 at 7:02 AM Jeff King <peff@peff.net> wrote: > > On Thu, Jul 02, 2020 at 04:08:43PM +0200, Christian Couder wrote: > > The other difference with V1 is that there are more tests in patch > > 2/2. These new tests required a small helper function to be > > introduced. > > I'm still not sure why %(objectsize) isn't sufficient here. Is there > some use case that's served by %(contents:size) that it wouldn't work > for? Or are we just trying to make it more discoverable when you're > looking at the contents already? %(objectsize) is the size of the whole commit or tag object, while %(contents:size) is the size of the complete message (the whole commit message or tag message, including trailers and signatures). $ git for-each-ref --format='refname: %(refname)%0a%09objectsize: %(objectsize)%0a%09contents:size: %(contents:size)' refs/heads/{master,next,seen} refs/tags/v2.2{5,6,7}.0 refname: refs/heads/master objectsize: 285 contents:size: 69 refname: refs/heads/next objectsize: 281 contents:size: 17 refname: refs/heads/seen objectsize: 589 contents:size: 325 refname: refs/tags/v2.25.0 objectsize: 974 contents:size: 842 refname: refs/tags/v2.26.0 objectsize: 974 contents:size: 842 refname: refs/tags/v2.27.0 objectsize: 974 contents:size: 842 When sending only the complete message, not the whole object, through a protocol, it might be interesting to easily get the length of the complete message. For example one could use "Content-Length: %(contents:size)%0a" when sending a complete message over HTTP.
On Tue, Jul 07, 2020 at 08:19:06AM +0200, Christian Couder wrote: > > I'm still not sure why %(objectsize) isn't sufficient here. Is there > > some use case that's served by %(contents:size) that it wouldn't work > > for? Or are we just trying to make it more discoverable when you're > > looking at the contents already? > > %(objectsize) is the size of the whole commit or tag object, while > %(contents:size) is the size of the complete message (the whole commit > message or tag message, including trailers and signatures). Ah, right, that makes sense. I'd probably use "git log --no-walk --format=%B" or similar for this, but there is nothing wrong with using for-each-ref (and it is better if you really do care about properties of the refs themselves, and not just the commit they point to). I do think in the long run it might be nice to have a generic placeholder for "expand this thing and give me the number of bytes", so we could do: %(sizeof:%(contents)) or even: %(sizeof:%(authorname) <%(authoremail)>) but that is definitely outside the scope. If we end up eventually with a generic mechanism and have to support contents:size forever for compatibility, I don't think it is that big a problem. -Peff