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

Patrick Steinhardt
2024-08-16T12:39:01Z

On Fri, Aug 16, 2024 at 11:35:33AM +0000, avih wrote:
> 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.

Great, thanks for your thorough explanations!

Patrick