Support `$foo/bar` in command position: parameter expansions with pasted constant strings

[segevfiner]

[phy1729]

Something like

diff --git a/highlighters/main/main-highlighter.zsh b/highlighters/main/main-highlighter.zsh
index 647eb8b..e905268 100644
--- a/highlighters/main/main-highlighter.zsh
+++ b/highlighters/main/main-highlighter.zsh
@@ -492,10 +492,14 @@ _zsh_highlight_highlighter_main_paint()
         local -a match mbegin mend
         local MATCH; integer MBEGIN MEND
         if [[ $res == none ]] && (( ${+parameters} )) &&
-           [[ ${arg[1]} == \$ ]] && [[ ${arg:1} =~ ^([A-Za-z_][A-Za-z0-9_]*|[0-9]+)$ ]] &&
+           [[ ${arg[1]} == \$ ]] && [[ ${${arg:1}%%/*} =~ ^([A-Za-z_][A-Za-z0-9_]*|[0-9]+)$ ]] &&
            (( ${+parameters[${MATCH}]} ))
            then
-          _zsh_highlight_main__type ${(P)MATCH}
+          if [[ $arg == */* ]]; then
+            _zsh_highlight_main__type ${(P)MATCH}/${arg#*/}
+          else
+            _zsh_highlight_main__type ${(P)MATCH}
+          fi
           res=$REPLY
         fi
       }

would work, but that's just special casing $var/string when many different characters can end a variable name.

I'm not sure this case should handled better. The current condition is a decent mix of correctness and simplicity. zsh-syntax-highlighting cannot just outsource the job of expanding parameters to zsh since parameter expansion can have side effects (e.g. ${foo:=bar}). Although I wonder if we can test for ( (to exclude glob qualifiers which can execute code) and if there's no ( expand $arg in a subshell.

[danielshahaf]

would work, but that's just special casing $var/string when many different characters can end a variable name.

It wouldn't be hard to rewrite it to support «$var.string», «$var,string», etc — strip the leading dollar-followed-by-alphanumerics and see what the rest of the string is — but then what about «$foo/$bar». In the end it's, as you say, a tradeoff between generality and simplicity (which relates to correctness, readability, etc).

It would probably be useful to highlight «$var:hey» as «$var:h» followed by «ey». I'll file a separate issue to investigate that.

Although I wonder if we can test for ( (to exclude glob qualifiers which can execute code) and if there's no ( expand $arg in a subshell.

Do you mean parameter expansion flags «${(foo)bar}» or glob qualifiers «./foo(bar)»?

I'm not sure off the top of my head, but in general, in such situations it's better to use a whitelist than a blacklist; i.e., not to blacklist '(' but to enumerate constructs that are explicitly known to work.

(For example, blacklisting '(' wouldn't block «${:-foo}», and if you block backticks you wouldn't be able to be sure that there isn't some third syntax that's problematic. Even allowing arbitrary parameter names could be problematic with parameters implemented by modules, e.g., imagine a module-provided $AUTOINCREMENT that increments by 1 every time it's eval'd.)

[danielshahaf]

Opened #485.

[phy1729]

I certainly prefer a whitelist, and agree it's the correct approach. From the $AUTOINCREMENT example it sounds like no parameters should ever be expanded since they could have side effects. I would not object to that and it's certainly a clearer line than what we have now or might have in the foreseeable future.

[danielshahaf]

$AUTOINCREMENT was a made-up example. Let's ask upstream's opinion.

[danielshahaf]

Posted users/23095. While writing that I realised that evaluating only if ${parameters[$foo]} != *special* would probably be safe (but let's see what the thread results in).

[danielshahaf]

Assigning this back to the pool: the $AUTOINCREMENT issue was handled by #489.

[danielshahaf]

Issue sweep: two separate concerns here:

1. Don't make the command word red when we don't know whether or not it'll execute successfully or not.
2. Support ${foo}/bar in command position where bar is a constant string.

We have #695 for (1) so I'm repurposing this issue to track (2). Patches welcome.

[danielshahaf]

Edge case to consider: highlighting $prefix/bin/zsh in command position when the variable $prefix is unset or empty and /bin/zsh exists.

I wonder if we can highlight $prefix as an elision (comment) and the remainder of the word in green. This would (1) differ from how that would be highlighted if $prefix were non-empty; (2) correctly indicate that the command-line is valid.