On Sat, Jul 20, 2024 at 6:37 PM Rubén Justo [off-list ref] wrote:

On Wed, Jul 17, 2024 at 04:09:17PM -0400, Eric Sunshine wrote:

quoted

quoted

quoted

quoted

-    test_write_lines P q | GIT_PAGER="head -n 1" test_terminal git add -p

quoted

It's also curious that t/check-non-portable-shell.pl didn't catch this
use of one-shot assignment when calling a shell function[*].

It would have been great if it had caught that error.

As a reference:
    VAR=value func           # this error is caught
    echo 1 |
    VAR=value func           # this one is also caught
    echo 1 | VAR=value func  # this one isn't

Thanks for providing this summary; it saved me the effort of digging
back through this discussion / patch series.

Maybe, catch this errors expanding the regular expression we have in
`check-non-portable-shell.pl` isn't the best approach.  We might need
something more sophisticated, like what we have in `chainlint.pl`.

The idea has been expressed previously of subsuming all the
check-non-portable-shell.pl checks into chainlint.pl some day, thus
allowing check-non-portable-shell.pl to be retired. In fact, it was
mentioned again quite recently[1].

However, this particular check (detecting `VAR=val shell-func`) poses
an extra complication which would require some specialized additional
mechanism in chainlint.pl. In particular, in `VAR=val symbol`, in
order to distinguish when `symbol` is an external command versus a
shell-function, it is necessary to scan for function definitions not
just in the script being checked, but also in all scripts included
(recursively) by the script being checked. So, it's probably possible
to do but ought to be done carefully.

Perhaps someone with experience in those scripts could give us this
capability :-)

I posted a series[2] which addresses this shortcoming by enhancing
check-non-portable-shell.pl.

[1]: https://lore.kernel.org/git/CAPig+cTFZuU7zM7poqk4HeK09zn8bFrO37eUZiaGmeJ0yecpiw@mail.gmail.com/ (local)
[2]: https://lore.kernel.org/git/20240722065915.80760-1-ericsunshine@charter.net/T/ (local)