On Fri, Jun 07, 2019 at 03:59:00PM -0700, Emily Shaffer wrote: > Teach show_object_with_name() to avoid writing a space before a name > which is empty. Also teach tests for rev-list --objects --filter to not > require a space between the object ID and name. > [...] > --- > I don't see any reason _not_ to remove this stray whitespace at the end, > since it seems like it just gets in the way of easy scripting. I also > think this case will only present itself for root trees. I'm a bit worried that this might break existing scripts. As ugly as trailing whitespace is, it does tell you something here: that the object is a root tree and not a commit. So in the past I have done things like: git rev-list --objects --all | grep ' ' to get only the non-commits. I'm undecided on whether we're straying into https://xkcd.com/1172/ territory here. I'd be more in favor if this were making things significantly easier, but... > show_object_with_name() inserts a space between an object's OID and name > regardless of whether the name is empty or not. This causes 'git > cat-file (--batch | --batch-check)' to fail to discover the type of root > directories: > > git rev-list --objects --filter=tree:1 --max-count=1 HEAD \ > | git cat-file --batch-check > git rev-parse HEAD: | xargs -I% git cat-file -t % > git rev-list --objects --filter=tree:1 --max-count=1 HEAD \ > | xargs -I% echo "AA%AA" > git rev-list --objects --filter=tree:1 --max-count=1 HEAD \ > | cut -f 1 -d ' ' | git cat-file --batch-check Your patch only helps with this at all because you're using the "tree:1" filter. It would not help: git rev-list --objects HEAD | git cat-file --batch-check because there you'll have actual names which cat-file will choke on. So it seems like this is helping only a very limited use case. cat-file actually does know how to split on whitespace. Unfortunately it does not do so by default, because that breaks some cases. See 97be04077f (cat-file: only split on whitespace when %(rest) is used, 2013-08-02). So you _can_ do: git rev-list --objects HEAD | git cat-file --batch-check='%(objectname) %(objecttype) %(rest)' But: 1. That puts the %(rest) bits in your output, which you may not want. 2. You have to actually specify the full format, so you might have to repeat batch-check's default format items. I think it would be reasonable for cat-file to have an option to split on whitespace (and if not given explicitly by the user, default to the presence of %(rest) as we do now). Alternatively, it would be reasonable to me to have an option for "rev-list --objects" to have an option to suppress the filename (and space) entirely. I think in the longer run along those lines that "--objects" should allow cat-file style pretty-print formats, which would eliminate the need to pipe to cat-file in the first place. That makes this parsing problem go away entirely, and it's way more efficient to boot (rev-list already knows the types!). -Peff
