Skip to content

word after "alias ssc='sudo systemctl'; ssc " displayed as error #552

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

Closed
blueyed opened this issue Oct 14, 2018 · 6 comments
Closed

word after "alias ssc='sudo systemctl'; ssc " displayed as error #552

blueyed opened this issue Oct 14, 2018 · 6 comments

Comments

@blueyed
Copy link

blueyed commented Oct 14, 2018

I have an alias ssc='sudo systemctl', and zsh-syntax-highlighting will display stop in red with ssc stop.

ssc ls will not display an error, and alias ssc=systemctl (without sudo) is also fine.

Bisected to f3410c5.

@danielshahaf
Copy link
Member

Currently only the first word of the alias's expansion is used. Fixing this in full generality is probably non-trivial, because alias foo='sudo systemctl &&' is also valid (and furthermore, the expansion of an alias can contain global aliases). I suppose we could at least hotfix this by allowing the :regular: state for the word after an alias, in order to prevent the false positive you're seeing... Thoughts?

@blueyed Feel free to join IRC too.

@blueyed
Copy link
Author

blueyed commented Oct 14, 2018

I understand that it uses sudo and then the second word is meant to be a command, from/via sudo's completion? Is that correctCould the completion system be used to get the type (after using the whole alias expansion)? Foralias foo='sudo systemctl &&'` it would be a command then.
(Sorry, I do nothing about the internals here)

@danielshahaf
Copy link
Member

z-sy-h doesn't use completion. That'd be a great thing to do (it would allow highlighting whole new classes of usage errors), but right now, z-sy-h simply parses $PREBUFFER$BUFFER by itself, using ${(z)}.

What the internals do is resolve the alias, look at the first word in the alias to decide what it is, and throw away the rest. Historically, it is this way because the alias is resolve inside the 'for each word' loop, but now that I look at it, I wonder if the alias resolving code could not throw those away?

diff --git a/highlighters/main/main-highlighter.zsh b/highlighters/main/main-highlighter.zsh
index 455c21f..e30e044 100644
--- a/highlighters/main/main-highlighter.zsh
+++ b/highlighters/main/main-highlighter.zsh
@@ -69,6 +69,7 @@ _zsh_highlight_highlighter_main_predicate()
 
 # Helper to deal with tokens crossing line boundaries.
 _zsh_highlight_main_add_region_highlight() {
+  (( skip_words == 0 )) || return 0
   integer start=$1 end=$2
   shift 2
 
@@ -395,14 +396,19 @@ _zsh_highlight_main_highlighter_highlight_list()
   # Processing buffer
   local proc_buf="$buf"
   local -a args
+  # How many words to skip (these words are the result of alias expansion and are not added to region_highlight)
+  integer skip_words=0
   if [[ $zsyh_user_options[interactivecomments] == on ]]; then
     args=(${(zZ+c+)buf})
   else
     args=(${(z)buf})
   fi
-  for arg in $args; do
+  while (( ${+args[1]} )); do
+    arg=$args[1]
+    args=( ${args[2,-1]} )
     # Save an unmunged copy of the current word.
     arg_raw="$arg"
+    (( skip_words <= 0 )) || (( --skip_words ))
 
     # Initialize this_word and next_word.
     if (( in_redirection == 0 )); then
@@ -516,6 +522,8 @@ _zsh_highlight_main_highlighter_highlight_list()
             # Avoid looping forever on alias a=b b=c c=b, but allow alias foo='foo bar'
             [[ $arg == $reply[1] ]] && break
             arg=$reply[1]
+            args[1,0]=( $reply[2,-1] )
+            (( skip_words += $#reply - 1 ))
             if (( $+seen_arg[$arg] )); then
               res=none
               break

This actually passes most tests, except for these:

# alias-assignment1
1..3
not ok 1 - [18,21] «» - expected (1 3 "unknown-token"), observed (18 21 "default").
not ok 2 - [22,23] «» - expected (5 6 "default"), observed (22 23 "default").
ok 3 - cardinality check
# alias-comment2
1..2
not ok 1 - [5,7] «» - expected (1 1 "unknown-token"), observed (5 7 "builtin").
ok 2 - cardinality check
# alias-precommand-option-argument
1..5
not ok 1 - [5,6] «ph» - expected (1 3 "alias"), observed (5 6 "single-hyphen-option").
not ok 2 - [7,13] «y1729 e» - expected (5 11 "default"), observed (7 13 "default"). # TODO "issue #540"
not ok 3 - [14,17] «cho » - expected (13 16 "commmand"), observed (14 17 "builtin"). # TODO "issue #540"
ok 4 - [18,20] «foo»
ok 5 - cardinality check
# alias
1..4
ok 1 - [1,8] «x.alias2»
ok 2 - [9,9] «;»
not ok 3 - [17,25] «» - expected (11 16 "alias"), observed (17 25 "default").
ok 4 - cardinality check

@danielshahaf
Copy link
Member

This is closer, but still wrong.

diff --git a/highlighters/main/main-highlighter.zsh b/highlighters/main/main-highlighter.zsh
index 455c21f..ec0979c 100644
--- a/highlighters/main/main-highlighter.zsh
+++ b/highlighters/main/main-highlighter.zsh
@@ -69,6 +69,7 @@ _zsh_highlight_highlighter_main_predicate()
 
 # Helper to deal with tokens crossing line boundaries.
 _zsh_highlight_main_add_region_highlight() {
+  (( skip_words == 0 )) || return 0
   integer start=$1 end=$2
   shift 2
 
@@ -395,14 +396,19 @@ _zsh_highlight_main_highlighter_highlight_list()
   # Processing buffer
   local proc_buf="$buf"
   local -a args
+  # How many words to skip (these words are the result of alias expansion and are not added to region_highlight)
+  integer skip_words=0
   if [[ $zsyh_user_options[interactivecomments] == on ]]; then
     args=(${(zZ+c+)buf})
   else
     args=(${(z)buf})
   fi
-  for arg in $args; do
+  while (( ${+args[1]} )); do
+    arg=$args[1]
+    shift args
     # Save an unmunged copy of the current word.
     arg_raw="$arg"
+    (( skip_words <= 0 )) || (( --skip_words ))
 
     # Initialize this_word and next_word.
     if (( in_redirection == 0 )); then
@@ -428,6 +434,7 @@ _zsh_highlight_main_highlighter_highlight_list()
     fi
 
     # Compute the new $start_pos and $end_pos, skipping over whitespace in $buf.
+    if (( skip_words == 0 )); then
     start_pos=$end_pos
     if [[ $arg == ';' ]] ; then
       # We're looking for either a semicolon or a newline, whichever comes
@@ -462,6 +469,7 @@ _zsh_highlight_main_highlighter_highlight_list()
       ((start_pos+=offset))
       ((end_pos=$start_pos+${#arg}))
     fi
+    fi
 
     # Compute the new $proc_buf. We advance it
     # (chop off characters from the beginning)
@@ -516,6 +524,8 @@ _zsh_highlight_main_highlighter_highlight_list()
             # Avoid looping forever on alias a=b b=c c=b, but allow alias foo='foo bar'
             [[ $arg == $reply[1] ]] && break
             arg=$reply[1]
+            args[1,0]=( "${(@)reply[2,-1]}" )
+            (( skip_words += $#reply - 1 ))
             if (( $+seen_arg[$arg] )); then
               res=none
               break

@tonylambiris
Copy link

This is happening for me as well -- seemed to have started very recently too (within the past week or so).
gnome-shell-screenshot-bgn4qz

@phy1729
Copy link
Member

phy1729 commented Oct 21, 2018

Fixed in cb8c736 #556.

@phy1729 phy1729 closed this as completed Oct 21, 2018
@phy1729 phy1729 added this to the 0.7.0 milestone Oct 21, 2018
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

4 participants