From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 21 Jul 2023 19:02:37 GMT
Subject: Re: [PATCH 2/5] t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()`
Message-ID: <xmqqfs5hulyq.fsf@gitster.g>
In-Reply-To: <ZLrQyZAoRIaga1mT@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

>>     graph_git_two_modes "${DIR:+-C \"$DIR\"} log ..." &&
>>
>> Unfortunately, however, some older broken shells incorrectly expand
>> this to a single argument ("-C <dir>") rather than the expected two
>> arguments (-C and "<dir>")[1,2,3,4]. The workaround is unsightly but
>> doable:
>>
>>     graph_git_two_modes "${DIR:+-C} ${DIR:+\"$DIR\"} log ..." &&
>
> Hmm. I get what you're saying, but I think in this case we're OK, since
> this all goes to `graph_git_two_modes`, whose implementation looks like:
>
>     graph_git_two_modes() {
>         git -c core.commitGraph=true $1 >output
>     }
>
> So I think we really do want everything smashed together into a single
> argument.

This is not a scripted Porcelain that must cope with any funnies the
end users may throw at us, after all.

How about adding a new prominent note at the beginning of
graph_git_behavior helper to declare that it is a bug if DIR
contains any characters (e.g. $IFS) that may make the shell
misbehave, and then stop worrying about quoting of this one?

That would keep the end result slightly more readable ;-)