| Message ID | pull.1750.git.git.1721762306.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | git-prompt: support more shells | expand |
On 2024-07-23 at 19:18:18, Avi Halachmi via GitGitGadget wrote: > Before this patchset only bash and zsh were supported. > > After this patchset, the following shells work: bash, zsh, dash (since at > least 0.5.8), free/net bsd sh, busybox-ash, mksh, openbsd sh, pdksh(!), > Schily extended Bourne sh (bosh), yash. > > The code should now be almost posix-compliant, with one big exception > ("local" variables in functions) which is not simple to fix. We explicitly allow `local` in our coding guidelines. As a side note, Debian requires it of all shells that can be used as `/bin/sh`. > Shells which don't work, likely only due to missing "local": ksh93[u+m], > Schily minimal posix Bourne sh (pbosh), yash-posix-mode. ksh93u+m is planning on adding local in a future revision.
On Wednesday, July 24, 2024 at 01:50:16 AM GMT+3, brian m. carlson <sandals@crustytoothpaste.net> wrote: > We explicitly allow `local` in our coding guidelines. Yeah. I missed the guidelines initially, but I got to the same conclusion with git-prompt.sh - to allow only "local" exception. > As a side note, > Debian requires it of all shells that can be used as `/bin/sh`. I wasn't aware of that, but it does makes sense to me. > ksh93u+m is planning on adding local in a future revision. That's nice. I did try to check whether it's planned, and request if it wasn't, but I didn't find the future plans (but also didn't try too hard). Though I think they're still doing bug fixes for the forseeable future, which is also great. Looking forward to it. However, while they do have typeset (in non-posix 'function foo()...), it has syntactic scope and not dynamic like with "local", so it wouldn't be a trivial mapping to an existing functionality. "local" is so useful, and with minimal application restrictions it's already effectively portable with very few exceptions, but ksh93[u+m] is indeed one of the notable ones. I think was a missed opportinity that POSIX 2024 didn't include "local" (I know it was discussed). It is possible to implement the functionality compliantly and even with reasonable syntax, no tricks, and very good performance, but it's not the same as being officially supported.
avih <avihpit@yahoo.com> writes: > On Wednesday, July 24, 2024 at 01:50:16 AM GMT+3, brian m. carlson <sandals@crustytoothpaste.net> wrote: > >> We explicitly allow `local` in our coding guidelines. > > Yeah. I missed the guidelines initially, but I got to the same > conclusion with git-prompt.sh - to allow only "local" exception. It is a bit more nuanced than that, though. Here is what we say: - Even though "local" is not part of POSIX, we make heavy use of it in our test suite. We do not use it in scripted Porcelains, and hopefully nobody starts using "local" before all shells that matter support it (notably, ksh from AT&T Research does not support it yet). For the purpose of git-prompt, I think it should be OK (without "local", it is harder, if not impossible, to clobber end-user's shell variable namespace with various temporaries we need to use during prompt computation) to declare that we now support shells other than bash and zsh as long as they are reasonably POSIX and support "local" that is dynamic. > That's nice. I did try to check whether it's planned, and request if > it wasn't, but I didn't find the future plans (but also didn't try > too hard). Though I think they're still doing bug fixes for the > forseeable future, which is also great. Looking forward to it. Do we know what kind of "local" is ksh93 adding? The same as their "typeset" that is not dynamic? That is so different from what others do and scripts expect to be all that useful, I am afraid.
On Wednesday, July 24, 2024 at 06:29:12 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote: > > It is a bit more nuanced than that, though. Here is what we say: > > - Even though "local" is not part of POSIX, we make heavy use of it > in our test suite. We do not use it in scripted Porcelains, and > hopefully nobody starts using "local" before all shells that matter > support it (notably, ksh from AT&T Research does not support it yet). > > For the purpose of git-prompt, I think it should be OK ... > to declare that we now support shells > other than bash and zsh as long as they are reasonably POSIX and > support "local" that is dynamic. I have to admit I missed "in our test suite". Right, so no "local" in Porcelains, but yes in the test suite. But yes, agreed, because it supports so many more shells. The commit "git-prompt: ta-da! document.." does document it. > (without > "local", it is harder, if not impossible, to clobber end-user's > shell variable namespace with various temporaries we need to use > during prompt computation) It actually is technically possible with git-prompt.sh, with 1 LOC. See the "anecdote" at end of the same "ta-da!" commit message which does exactly that. Though for obvious reason we can't really do that. > Do we know what kind of "local" is ksh93 adding? The same as their > "typeset" that is not dynamic? That is so different from what others > do and scripts expect to be all that useful, I am afraid. I would think it has to be similar enough to other shells, or else it creates a compatibility nightmare IMO. But that's a guess. Somewhat off topic, so apologies if this shouldn't be here: As for the Porcelains, I have to assume that it can be unpleasant to maintain big scripts without "local". Would there be an interest in adding a facility with the same semantics as "local", but posix compliant (and also posix-ish shells), for use in Porcelains? It's not a drop-in replacement, but the syntax is reasonable IMO: locals myfunc x y _myfunc () { echo "$? $1 $2" x=1 y=2 return 33 } x=x; unset y (exit 42) || myfunc foo bar echo "$? $x ${y-unset}" Prints: 42 foo bar 33 x unset "locals myfunc x y" creates a wrapper function "myfunc" which saves the state of $x and $y, calls _myfunc "$@", then restores the state (and propagates the initial and final $? appropriately). Recursion is supported, the wrapper doesn't create additional variables, and no subshells are used at the wrapper (also not at "locals"). The implementation of "locals" is small (10-20 LOC), but we can't expect scripts to embed it, so it would need to be sourced (dot). If there is interest in such thing, let me know, and I can submit a patch (independent of this patchset) to adds such file which can then be sourced by other scripts in order to use "locals". Unrelated, and it might not mean much, but I did want to thank you for maintaining git all those years.
Junio C Hamano <gitster@pobox.com> writes: > For the purpose of git-prompt, I think it should be OK (without > "local", it is harder, if not impossible, to clobber end-user's > shell variable namespace with various temporaries we need to use > during prompt computation) to declare that we now support shells > other than bash and zsh as long as they are reasonably POSIX and > support "local" that is dynamic. "to clobber" -> "to avoid clobbering", sorry for the noise.
Before this patchset only bash and zsh were supported. After this patchset, the following shells work: bash, zsh, dash (since at least 0.5.8), free/net bsd sh, busybox-ash, mksh, openbsd sh, pdksh(!), Schily extended Bourne sh (bosh), yash. The code should now be almost posix-compliant, with one big exception ("local" variables in functions) which is not simple to fix. Shells which don't work, likely only due to missing "local": ksh93[u+m], Schily minimal posix Bourne sh (pbosh), yash-posix-mode. Most changes are trivial, like changing [[...]] to [...], with one exception (git-prompt: don't use shell arrays) which changes few lines. Tested with and without colors, with diversions from upstream, in git-svn repos, and more, but I'm not considering it full coverage. * avih Avi Halachmi (:avih) (8): git-prompt: use here-doc instead of here-string git-prompt: fix uninitialized variable git-prompt: don't use shell arrays git-prompt: replace [[...]] with standard code git-prompt: add some missing quotes git-prompt: add fallback for shells without $'...' git-prompt: ta-da! document usage in other shells git-prompt: support custom 0-width PS1 markers contrib/completion/git-prompt.sh | 196 +++++++++++++++++++++---------- 1 file changed, 131 insertions(+), 65 deletions(-) base-commit: d19b6cd2dd72dc811f19df4b32c7ed223256c3ee Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1750%2Favih%2Fprompt-compat-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1750/avih/prompt-compat-v1 Pull-Request: https://github.com/git/git/pull/1750