On Tue, Jun 14, 2022 at 09:09:11AM +0000, Brad Forschinger via GitGitGadget wrote: > From: Brad Forschinger <bnjf@xxxxxxxxxx> > > The test and [ commands are used throughout the prompt generation. They > also happen to be valid function names that can be defined, leading to > unintentional results. Prevent the somewhat unusual case of this > happening by simply using [[, which is reserved. Hmm. I do think we need to be a bit more paranoid about style in the prompt and completion code, because they are sourced into the user's shell alongside whatever other weird customizations they'd have. So we already have adjustments to work under "set -u", and so forth. But at some point we may say "you have made the environment too hostile for us to function". Is redefining "test" to something that doesn't behave the same way such a case? Part of me wants to say yes. :) That said, if it's not _hard_ to support, maybe it is worth doing to be on the cautious side? A few thoughts: - my biggest concern on cost is that this is an unusual style for our project (which usually writes in POSIX shell, though of course this file is meant to be bash/zsh specific). Will it be a maintenance burden going forward? - this only changes git-prompt.sh; doesn't the completion code have the same issue? - I don't write much bash-specific code, but I seem to recall that "[[" has some subtle differences to "[". Is it sufficiently a superset that these conversions are all equivalent? I think some like: > - if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then > + if [[ $pcmode = yes ]] && [[ $ps1_expanded = yes ]]; then are not equivalent, but it's an actual improvement (bash's builtin "[[" isn't confused by unquoted empty variables), but I don't know if there may be other gotchas. (I doubt this is an actual bug in the current code, as $pcmode always seems to be set, but just a more defensive style). -Peff