PR #776 fixed an issue with complex aliases and expansion. However, this change
also introduced a problem with aliases which contain `]` (for example, commonly
seen on macOS: `alias ]=open`), due to using an associative array `seen_alias`,
indexed by the alias name. Due to `"$seen_alias[$arg]"`, it would fail when
`$arg` is expanded to anything containing `]`'. Thus, typing `] /` would result
in:
```
> ] /
(anon):unset:3: seen_alias[]]: invalid parameter name
```
This change fixes the issue by ensuring we properly access keys in the
associative array `seen_alias`.
Older versions of zsh have issues with map keys having special
characters, especially lacking ways to remove such keys. The
issue is described in detail in
https://unix.stackexchange.com/questions/626393/in-zsh-how-do-i-unset-an-arbitrary-associative-array-element.
This fix uses proposal from
[zsh-workers/43269](https://www.zsh.org/mla/workers/2018/msg01073.html),
discovered by Stephane Chazelas, that boils down to avoid removing keys
from the map, and reconstruct the map anew with some keys omitted.
Co-authored-by: @phy1729
See comments within for the rationale.
This is a regression test for a regression that was only present in development
versions of PR #764 and was never present in master.
% repeat 3 { zsh -f tests/test-zprof.zsh main | tee … | grep -w _zsh_highlight | head -n1 }
18) 1 26895.86 26895.86 100.00% 6.35 6.35 0.02% _zsh_highlight
19) 1 27399.11 27399.11 100.00% 5.52 5.52 0.02% _zsh_highlight
19) 1 27027.58 27027.58 100.00% 5.66 5.66 0.02% _zsh_highlight
----
This commit has been rebased. The above statistics were measured after
the rebase. The below statistics had been measured before the rebase.
num calls time self name
-----------------------------------------------------------------------------------
1) 3 25689.17 8563.06 98.15% 18422.01 6140.67 70.38% _zsh_highlight_main_highlighter_highlight_list
2) 32390 5706.13 0.18 21.80% 2315.68 0.07 8.85% _zsh_highlight_main_highlighter_highlight_argument
19) 1 26173.33 26173.33 100.00% 5.27 5.27 0.02% _zsh_highlight
Interestingly, if I make the change in this diff to
_zsh_highlight_main_highlighter_highlight_double_quote —
> diff --git a/highlighters/main/main-highlighter.zsh b/highlighters/main/main-highlighter.zsh
> index da6ab2b..bb17618 100644
> --- a/highlighters/main/main-highlighter.zsh
> +++ b/highlighters/main/main-highlighter.zsh
> @@ -1462,10 +1462,13 @@ _zsh_highlight_main_highlighter_highlight_double_quote()
> local i j k ret style
> reply=()
>
> - for (( i = $1 + 1 ; i <= $#arg ; i += 1 )) ; do
> + (( i = $1 ))
> + while (( ++i <= $#arg )); do
> + i=${arg[(ib.i.)[\"\`\$\\\\${histchars[1]}]]}
> (( j = i + start_pos - 1 ))
> (( k = j + 1 ))
> case "$arg[$i]" in
> + ("") break;;
> ('"') break;;
> ('`') saved_reply=($reply)
> _zsh_highlight_main_highlighter_highlight_backtick $i
— it actually makes things measurably slower (!), even on input that has
a large number of pasted double-quoted strings: on «BUFFER=": ${(r.8*1500..foo"bar".):-}"»
the slowdown is (1123.24ms / 1091.06ms = 1.0295). Therefore, I won't be
committing that change.
% git co HEAD^ && repeat 3 { zsh -f tests/test-zprof.zsh main | tee … | grep -w _zsh_highlight | head -n1 }
HEAD is now at 64e3651 'main': Optimize a hot path.
19) 1 28765.13 28765.13 100.00% 5.57 5.57 0.02% _zsh_highlight
19) 1 28566.46 28566.46 100.00% 5.91 5.91 0.02% _zsh_highlight
19) 1 28248.12 28248.12 100.00% 5.57 5.57 0.02% _zsh_highlight
----
This commit has been rebased. The above statistics were measured after
the rebase. The below statistics had been measured before the rebase.
Before this patch:
num calls time self name
-----------------------------------------------------------------------------------
1) 3 33410.81 11136.94 98.51% 19277.07 6425.69 56.84% _zsh_highlight_main_highlighter_highlight_list
19) 1 33916.21 33916.21 100.00% 5.27 5.27 0.02% _zsh_highlight
With this patch:
num calls time self name
-----------------------------------------------------------------------------------
1) 3 27167.49 9055.83 98.17% 18754.77 6251.59 67.77% _zsh_highlight_main_highlighter_highlight_list
19) 1 27674.40 27674.40 100.00% 5.39 5.39 0.02% _zsh_highlight
And if test-zprof.zsh is changed to not set interactivecomments:
num calls time self name
-----------------------------------------------------------------------------------
1) 13360 36029.12 2.70 83.56% 30304.23 2.27 70.28% _zsh_highlight_main_highlighter_highlight_argument
21) 1 43117.76 43117.76 100.00% 4.52 4.52 0.01% _zsh_highlight
num calls time self name
-----------------------------------------------------------------------------------
1) 13360 14782.89 1.11 68.12% 9163.42 0.69 42.23% _zsh_highlight_main_highlighter_highlight_argument
21) 1 21699.93 21699.93 100.00% 4.17 4.17 0.02% _zsh_highlight
In those versions of zsh, «[[ -o nosuchoption ]]» is regarded as
a syntax error. In newer zsh versions, it merely returns non-zero
(specifically, it returns 3, unlike «[[ -o unsetoption ]]» which
returns 1).
Fixes#732.
Fixes#733.
As described in the last commit's log message, ${parameter_name_pattern]
explicitly matches positional parameters but ${parameters[$MATCH]}
expands to nothing in that case (when, e.g., [[ $MATCH == '1' ]]; note
this is equality of strings, not integers).
As a side effect, this removes the dependency on the zsh/parameter
module for expanding parameters.
On the test case, the behaviour was as follows:
+highlighters/main/main-highlighter.zsh:733> _zsh_highlight_main_highlighter__try_expand_parameter '$1'
+highlighters/main/main-highlighter.zsh:432> local arg='$1'
+highlighters/main/main-highlighter.zsh:433> unset reply
+highlighters/main/main-highlighter.zsh:439> local -a match mbegin mend
+highlighters/main/main-highlighter.zsh:440> local MATCH
+highlighters/main/main-highlighter.zsh:440> integer MBEGIN MEND
+highlighters/main/main-highlighter.zsh:441> local parameter_name
+highlighters/main/main-highlighter.zsh:442> local -a words
+highlighters/main/main-highlighter.zsh:443> [[ '$' != \$ ]]
+highlighters/main/main-highlighter.zsh:446> [[ 1 == { ]]
+highlighters/main/main-highlighter.zsh:449> parameter_name=1
+highlighters/main/main-highlighter.zsh:451> [[ none == none ]]
+highlighters/main/main-highlighter.zsh:451> zmodload -e zsh/parameter
+highlighters/main/main-highlighter.zsh:452> [[ ${parameter_name} -regex-match ^${~parameter_name_pattern}$ ]]
+highlighters/main/main-highlighter.zsh:453> [[ '' != *special* ]]
+highlighters/main/main-highlighter.zsh:456> case array-special (*array*|*assoc*)
+highlighters/main/main-highlighter.zsh:458> words=( '$1' )
+highlighters/main/main-highlighter.zsh:469> reply=( '$1' )
There are two problems here:
- In terms of _zsh_highlight_main_highlighter__try_expand_parameter's
pre- and postconditions, the expansion of the word «$1» (line 733)
included that same word (line 469).
That happened because word-to-be-expanded is passed to
_zsh_highlight_main_highlighter__try_expand_parameter as its first
positional parameter, and in this case the word happened to be «$1».
- Furthermore, the exclusion of special parameters (line 453) false
negatived. That happened because $parameter_name_pattern explicitly
allows positional parameters, but ${parameters[(e)1]} expands to
nothing. This will be fixed in the next commit.
Not a regression from 0.7.1.
Before this commit, if the value didn't begin with a dollar sign,
_zsh_highlight_main_highlighter__try_expand_parameter() would return 1
by accident.¹ Tweak the input validation to make this behaviour
explicit. No functional change.
¹ Specifically, it would return 1 because ${parameter_name}'s value
would be the empty string and ${parameter_name_pattern} wouldn't match
that.
