Re: [PATCH] t2400: Fix test failures when using grep 2.5

> Jacob Abel <jacobabel@nullpo.dev> writes:
> 
> >> > @@ -998,8 +998,8 @@ test_dwim_orphan () {
> >> >   					headpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&
> >>
> >> I'm a bit confused by the --sq here - why does it need to be shell
> >> quoted when it is always used inside double quotes?
> >
> > To be honest I can't remember if this specifically needs to be in
> > quotes or not however I had a lot of trouble during the development of
> > that patchset with things escaping quotes and causing breakages in the
> > tests so if it isn't currently harmful I'd personally prefer to leave
> > it as is.
> 
> Quoting is sometimes tricky enough that "this happens to work for me
> but I do not know why it works" is asking for trouble in somebody
> else's environment.  If the form in the patch is correct, but tricky
> for others to understand, you'd need to pick it apart and document
> how it works (and if you cannot do so, ask for help by somebody who
> can, or simplify it enough so that you can explain it yourself).

> 
>     headpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&
> 
> In this case, "--sq" is a noop that only confuses readers, I think,
> and I would drop it if I were you.  "--git-path HEAD" is given by
> this call chain:
> 
>    builtin/rev-parse.c:cmd_rev_parse()
>    -> builtin/rev-parse.c:print_path()
>       -> transform path depending on the path format
>          -> puts()
> 
> and nowhere in this chain "output_sq" (which is set by "--sq") is
> even checked.  The transformations are all about relative, prefix,
> etc., and never about quoting.

> 
> The original test script t2400 (before your patch) does look crappy
> with full of long lines and coding style violations (none of which
> is your fault), and it may need to be cleaned up once this patch
> settles.
> 
> Thanks.