Re: [PATCH v2 3/8] git-prompt: don't use shell arrays

avih
2024-08-16T11:57:03Z

On Friday, August 16, 2024 at 01:52:11 PM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
>On Fri, Aug 16, 2024 at 09:53:36AM +0000, avih wrote:
>>  On Friday, August 16, 2024 at 11:50:14 AM GMT+3, Patrick Steinhardt <ps@pks.im> wrote:
>> > On Thu, Aug 15, 2024 at 01:14:08PM +0000, Avi Halachmi (:avih) via GitGitGadget wrote:
>> >>
>> >> -            svn_remote[$((${#svn_remote[@]} + 1))]="$value"
>> >> +            svn_remotes=${svn_remotes}${value}${LF}  # URI\nURI\n...
>> >
>> >
>> > I was wondering whether this is something we want to quote, mostly
>> > because I still have the failures of dash in mind when assigning values
>> > with spaces to a `local` variable without quoting. I do not know whether
>> > the same issues also apply to non-local variables though, probably not.
>>
>> IFS field splitting and glob expansion strictly never happen and never
>> happened at the assignment part of a "simple command", since the first
>> version of POSIX in 1994, so quotes are not needed to avoid that.
>
> That's the theory, yes. But as said, we did hit bugs in similar areas in
> dash where that wasn't properly honored, as Junio also pointed out on a
> later patch. But that was in non-POSIX area anyway, as to the best of my
> knowledge it only happens with `local` assignments.

Yes. "local" is special, and not only because it's not POSIX.

The difference with "local" is that it takes assignment as arguments.

A "simple command" (posix term) is composed of optional assignment[s]
and optional command (and arguments).

The assigments part is never IFS-split or glob-expanded, while the
command and arguments part is (in words which include unquoted
expansion or substitution) and therefore needs quotes, e.g.:

foo=$x bar=$y echo a="$b" c="$d"

There are other commands (beyond "local") which take assignment[s]
as arguments, like "export", "readonly" and "command".

Before posix 2024, these commands also required quoting of the
arguments-assignments - just like "local" needed in dash.

But posix 2024 introduced the concept of a "declaration utility"
(which takes assignments as arguments, like export, readonly, etc),
and the concept of "assignment context" where IFS-split and glob
expansion don't happen - like the assignment part of a simple
command, but now also in the assignment arguments of declaration
utilities.

And indeed, new versions of shells now don't need quotes in export
etc, and shells now make "local" a declaration utility which
doesn't need quotes of the assignment args, including in dash.

However, the reason we do use quotes in local, export, etc, is
because many instances of shells which don't yet (or will ever)
support it still exist, so we quote for compatibility with those,
but still it's only needed in assignments which are arguments to
commands - not in the assignment part of a simple command.

I've also updated the wording a bit of my guidelines in part 5/8,
and I'll include it at the commit message of 5/8 v3.