Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zsh-syntax-highlighting does not like the alias '=' #263

Closed
Ndolam opened this issue Jan 6, 2016 · 8 comments
Closed

zsh-syntax-highlighting does not like the alias '=' #263

Ndolam opened this issue Jan 6, 2016 · 8 comments

Comments

@Ndolam
Copy link

Ndolam commented Jan 6, 2016

I have a function called '=' and an alias for = which is 'noglob ='.
(I find it very useful to be able to type things like
= 417 99 2 k /
and have it tell me
4.21
)
and '=' not only seems like a nice alias for this, but is what I used for many years with tcsh before I started using zsh.

Anyway, there is a line in main-highlighter.zsh
local aliased_command="${"$(alias -- $arg)"#*=}"
which gets upset if you have an alias like '='. However, the line
local aliased_command="$aliases[$arg]"
seems to work for = and other aliases (in my tests), and is (arguably)
a bit more succinct and clearer.

Would you consider including it in the main branch?

Thanks.

                            Jim
@danielshahaf
Copy link
Member

Anyway, there is a line in main-highlighter.zsh
local aliased_command="${"$(alias -- $arg)"#*=}"
which gets upset if you have an alias like '='.

Can you show us the code that defines the alias?

Is it aliases[=]='noglob "="'? The first suspect,alias \==foo, errors out with zsh: bad assignment (I think bin_alias splits on the first = sign and checks that the resulting LHS is non-empty).

However, the line
local aliased_command="$aliases[$arg]"
seems to work for = and other aliases (in my tests), and is (arguably)
a bit more succinct and clearer.

Would you consider including it in the main branch?

I would. The potential downside is that it requires the zsh/parameter module, but I think in practice everyone has that module... so, I propose we start requiring the module, and if somebody then speaks up saying their zsh doesn't have the module, revisit the decision.

Thanks for the report!

@danielshahaf
Copy link
Member

Added the Upstream label since this may be related to upstream issues around creation of aliases with = in their LHSes.

@Ndolam
Copy link
Author

Ndolam commented Jan 7, 2016

On Wed, Jan 6, 2016 at 14:27 (-0800), Daniel Shahaf wrote:

Anyway, there is a line in main-highlighter.zsh
local aliased_command="${"$(alias -- $arg)"#*=}"
which gets upset if you have an alias like '='.

Can you show us the code that defines the alias?

Is it aliases[=]='noglob "="'?
Close. I use
aliases[=]='noglob ='
without the double quotes.

The first suspect,alias \==foo, errors out with zsh: bad assignment (I think bin_alias splits on the first = sign and
checks that the resulting LHS is non-empty).
Yeah, I tried a few things related to quoting the = when I switched
from tcsh before giving up and asking on some list.

However, the line
local aliased_command="$aliases[$arg]"
seems to work for = and other aliases (in my tests), and is (arguably)
a bit more succinct and clearer.

Would you consider including it in the main branch?

I would. The potential downside is that it requires the
zsh/parameter module,
Good point. I forgot that I needed that to do what I do.

but I think in practice everyone has that module... so, I propose we
start requiring the module, and if somebody then speaks up saying
their zsh doesn't have the module, revisit the decision.
And if you do require it, there may be some other places where you can
simplify expressions.

Thanks for the report!
You're welcome. Thanks for the quick response.

Cheers.

                            Jim

@danielshahaf
Copy link
Member

This patch makes the error (after creating = as an alias and typing it in command position) go away:

diff --git a/highlighters/main/main-highlighter.zsh b/highlighters/main/main-highlighter.zsh
index bc6e2bb..5c1ceb9 100644
--- a/highlighters/main/main-highlighter.zsh
+++ b/highlighters/main/main-highlighter.zsh
@@ -295,7 +295,13 @@ _zsh_highlight_main_highlighter()
                         style=$ZSH_HIGHLIGHT_STYLES[suffix-alias]
                         ;;
         *': alias')     style=$ZSH_HIGHLIGHT_STYLES[alias]
-                        local aliased_command="${"$(alias -- $arg)"#*=}"
+                        local aliased_command
+                        if zmodload -e zsh/parameter; then
+                          (( $+aliases )) || { zle -M "$0: zsh/parameter does not define \$aliases" }
+                          aliased_command=${aliases[$arg]}
+                        else
+                          aliased_command="${"$(alias -- $arg)"#*=}"
+                        fi
                         [[ -n ${(M)ZSH_HIGHLIGHT_TOKENS_PRECOMMANDS:#"$aliased_command"} && -z ${(M)ZSH_HIGHLIGHT_TOKENS_PRECOMMANDS:#"$arg"} ]] && ZSH_HIGHLIGHT_TOKENS_PRECOMMANDS+=($arg)
                         ;;
         *': builtin')   style=$ZSH_HIGHLIGHT_STYLES[builtin];;

Remaining work:

  • Check with upstream whether aliases[foo=bar]=baz (where foo and bar may be empty) is supposed to work. (I.e., whether the disparity with the alias builtin is a bug).
  • Add regression test (@Ndolam, you're welcome to do that if you want — see tests/README.md — but you might want to wait until we're sure upstream doesn't consider the behaviour on aliases whose LHSes contain = a bug and changes it in 5.3)
  • Check whether other places in the code could benefit from presence of zsh/parameter (or whether zsh/parameter came up in previous issues)
  • Update docs to recommend or require zmodload zsh/parameter.

@danielshahaf
Copy link
Member

  • Check with upstream whether aliases[foo=bar]=baz (where foo and bar may be empty) is supposed to work. (I.e., whether the disparity with the alias builtin is a bug).

Upstream says (in workers/37545) that assignments to $aliases directly are not supported, they may break some functionality, if it breaks you get to keep both pieces.

@Ndolam
Copy link
Author

Ndolam commented Jan 11, 2016

On Sun, Jan 10, 2016 at 12:28 (-0800), Daniel Shahaf wrote:

  • Check with upstream whether aliases[foo=bar]=baz (where foo and bar may be empty) is supposed to work. (I.e., whether the disparity with the alias builtin is a bug).

Upstream says (in workers/37545) that assignments to $aliases directly are not supported, they may break some functionality, if it breaks you get to keep both pieces.

That's true, although the given example of trying to alias "x=y" to
something is a bit more of a pathological case, since that won't be
parsed as a valid alias anyway.

Anyway, thanks again for looking at it.

                            Jim

@danielshahaf
Copy link
Member

That's true, although the given example of trying to alias "x=y" to something is a bit more of a pathological case, since that won't be parsed as a valid alias anyway.

Yes and no. Trying to run the command x=y ls doesn't look up x=y as an alias, but splitting that same command line by ${(z)…} does yield x=y as a single word, hence the above patch would highlight x=y as an alias (which would be a false positive).

@danielshahaf
Copy link
Member

This will happen as part of #298.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants