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

> On Fri, Jun 7, 2019 at 6:59 PM Emily Shaffer <emilyshaffer@google.com> 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.
> > [...]
> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > ---
> > Not sure if making the whitespace optional is the right solution for the
> > test, although I couldn't come up with a cleaner approach. Maybe
> > something like this would be better, even though handling it in the
> > regex is shorter?
> >
> >   if [[ -z "$name" ]] then
> 
> In Git, we avoid Bash-isms. Just use 'test'. And, style is to place
> 'then' on its own line.
> 
>     if test -z "$name"
>     then
> 
> > diff --git a/revision.c b/revision.c
> > @@ -40,7 +40,8 @@ void show_object_with_name(FILE *out, struct object *obj, const char *name)
> >  {
> > -       fprintf(out, "%s ", oid_to_hex(&obj->oid));
> > +       fprintf(out, "%s%s", oid_to_hex(&obj->oid),
> > +               strcmp(name, "") == 0 ? "" : " ");
> >         for (p = name; *p && *p != '\n'; p++)
> >                 fputc(*p, out);
> >         fputc('\n', out);
> 
> It's subjective, but this starts to be a bit too noisy and unreadable.
> An alternative:
> 
>     fputs(oid_to_hex(...), out);
>     if (*name)
>         putc(' ', out);
> 
> > diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
> > @@ -288,7 +288,7 @@ expect_has () {
> >         hash=$(git -C r3 rev-parse $commit:$name) &&
> > -       grep "^$hash $name$" actual
> > +       grep "^$hash \?$name$" actual
> 
> This would be the first use of \? with 'grep' in the test suite. I
> would worry about portability. Your suggestion earlier to tailor
> 'grep' invocation based upon whether $name is empty would be safer.