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

Autoload and emulate zsh #758

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

phy1729
Copy link
Member

@phy1729 phy1729 commented Aug 10, 2020

Since redrawhook is now merged, figured I'd start the autoload highlighters and emulate zsh source driver branch.

Still appears to work locally (including setopt interactivecomments and then typing a line with comments). I'm running 5.8 though, so tests with a dev version with redrawhook active are needed.

Related issues: #252 #536 #688

@phy1729 phy1729 force-pushed the autoload branch 2 times, most recently from 6449f17 to d38ba1d Compare August 10, 2020 04:01
@phy1729
Copy link
Member Author

phy1729 commented Aug 10, 2020

Tests pass on 3423a97 driver: Split out autoloadable part of _zsh_highlight, but fail on ef754de driver: Run under emulate zsh. I believe this is due to defining a function under emulate being sticky, but will need to further investigate later.

@danielshahaf
Copy link
Member

danielshahaf commented Aug 10, 2020 via email

@danielshahaf
Copy link
Member

danielshahaf commented Aug 10, 2020 via email

@danielshahaf
Copy link
Member

Skimmed. It's very comprehensive. Much kudos! Let me know how I can help.

It's an invasive change so I'll definitely prefer to merge it before a release rather than after, to ease backports, should any be needed. I take it you're targeting this branch for 0.8.0. (I don't have a strong opinion one way or the other.)

@phy1729 phy1729 force-pushed the autoload branch 3 times, most recently from 5e45f0f to a8b9480 Compare August 11, 2020 00:27
@danielshahaf danielshahaf added this to the 0.8.0 milestone Aug 11, 2020
@phy1729
Copy link
Member Author

phy1729 commented Aug 12, 2020

Upon further testing, while tests may pass, interactively options that are reset by emulate zsh (e.g. equals) are reset by the callers of _zsh_highlight (in particular _zsh_highlight_call_widget pre-redrawhook. I've moved the required code back into zsh-syntax-highlighting.zsh so that _zsh_highlight still executes under the user's environment.

Still need to add the fallback code if :P isn't available, but I think that's it, so it's testable now.

@danielshahaf
Copy link
Member

Travis is failing on pre-5.3 zsh with several different warnings. I didn't check whether some of these happen on 5.3-and-later too, which are green.

Running test pattern
# global (driver) tests
1..1
./driver.zsh:32: unrecognized modifier `P'
zsh:122: command not found: is-at-least
ok 1 # 'alias -- +foo=bar' is preserved
(anon):55: array parameter ZSH_HIGHLIGHT_HIGHLIGHTERS created globally in function (anon)
(anon):243: array parameter ZSH_HIGHLIGHT_STYLES created globally in function (anon)
1..2
## rm-rf
# BUFFER=$'rm -rf /'
not ok 1 - unmatched expectation (1 8 fg=white,bold,bg=red) 
todo=''
not ok 2 - cardinality check - have 1 expectations and 0 region_highlight entries: «expected_region_highlight=( $'1 8 fg=white,bold,bg=red' )» «region_highlight=(  )»
# expected_region_highlight   region_highlight
# '1 8 fg=white,bold,bg=red'
/work/./highlighters/pattern/test-data/rm-rf.zsh:30: array parameter ZSH_HIGHLIGHT_PATTERNS created globally in function (anon)
_zsh_highlight:51: command not found: _zsh_highlight_internal
Bail out! On './highlighters/pattern/test-data/rm-rf.zsh': output on stderr
Running test regexp
# global (driver) tests
1..1
./driver.zsh:32: unrecognized modifier `P'
zsh:122: command not found: is-at-least
ok 1 # 'alias -- +foo=bar' is preserved
(anon):55: array parameter ZSH_HIGHLIGHT_HIGHLIGHTERS created globally in function (anon)
(anon):243: array parameter ZSH_HIGHLIGHT_STYLES created globally in function (anon)
1..3
## complex
# BUFFER=$'echo 1+9-3 7+2'
not ok 1 - unmatched expectation (6 10 fg=white,bold,bg=red) 
not ok 2 - unmatched expectation (12 14 fg=white,bold,bg=red) 
todo=''
not ok 3 - cardinality check - have 2 expectations and 0 region_highlight entries: «expected_region_highlight=( $'6 10 fg=white,bold,bg=red' $'12 14 fg=white,bold,bg=red' )» «region_highlight=(  )»
# expected_region_highlight     region_highlight
# '6 10 fg=white,bold,bg=red'
# '12 14 fg=white,bold,bg=red'
/work/./highlighters/regexp/test-data/complex.zsh:30: array parameter ZSH_HIGHLIGHT_REGEXP created globally in function (anon)
_zsh_highlight:51: command not found: _zsh_highlight_internal
Bail out! On './highlighters/regexp/test-data/complex.zsh': output on stderr
Makefile:39: recipe for target 'test' failed
make: *** [test] Error 2
travis_time:end:03040b10:start=1597206903543376108,finish=1597206911136524385,duration=7593148277,event=script

The command "docker run -v $PWD:/work -w /work zshusers/zsh:${ZSH} /bin/sh -c 'install_packages make procps bsdmainutils && make test'" exited with 2.

@danielshahaf
Copy link
Member

Tested under zsh zsh-5.8-177-gd14a924 and zsh-5.8-241-g7facca6. All tests pass under both. Starting the latter interactively, sourcing z-sy-h, unsetting EQUALS, then typing echo foo =ls bar, results in =ls being underlined, though it shouldn't be. What else should I test?

@issue-dispenser
Copy link

@danielshahaf @phy1729 What are the remaining blockers for this to be merged?

@phy1729 phy1729 modified the milestones: 0.8.0, 0.8.1 Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants