Re: [PATCH] revision: remove stray whitespace when name empty

[Jeff King <peff@peff.net>]

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