Jacob Abel <jacobabel@xxxxxxxxxx> 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.
