zdharma-continuum / zinit

🌻 Flexible and fast ZSH plugin manager
MIT License
2.95k stars 126 forks source link

fix: Do not try to escape exclamation marks #399

Closed jankatins closed 1 year ago

jankatins commented 1 year ago

For normal hooks, the exclamation mark handling (which adds entries into ZINIT_EXTS2) was fixed in https://github.com/zdharma-continuum/zinit/pull/227 and for both ZINIT_EXTS and ZINIT_EXTS2 the escaped exclamation marks were adjusted. But annexes could also register hooks with exclamation marks into ZINIT_EXTS and this part was not modified. Therefore annex registering for exclamation mark hooks did not work anymore.

Description

See above + the part where ZINIT_EXTS is modified is now also adjusted in a similar manner as in #227 to not expand history chars (=exclamation marks).

Motivation and Context

See https://github.com/zdharma-continuum/zinit-annex-patch-dl/issues/6

Related Issue(s)

See https://github.com/zdharma-continuum/zinit-annex-patch-dl/issues/6

Usage examples

The following three prints should all three print the same (two) lines (espcially no escaped exclamation mark!) when the dl annex is used in .zshrc:

zsh -i -c 'print -rC1 -- "$ZINIT_EXTS[@]" |sort | grep "patch-dl"'
zsh -c 'source .zshrc && print -rC1 -- "$ZINIT_EXTS[@]" |sort | grep "patch-dl"'
exec zsh 
print -rC1 -- "$ZINIT_EXTS[@]" |sort | grep "patch-dl"

Example:

λ  print -rC1 -- "$ZINIT_EXTS[@]" |sort | grep 'patch-dl'
11 z-annex-data: zinit-annex-patch-dl hook:!atclone-20 za-patch-dl-handler za-patch-dl-help-null-handler dl\'\'\|patch\'\'
12 z-annex-data: zinit-annex-patch-dl hook:!atpull-20 za-patch-dl-handler za-patch-dl-help-null-handler ''

How Has This Been Tested?

λ  zinit delete dltest -y

Done (action executed, exit code: 0)

λ  zi for id-as'dltest' as'null' dl'https://raw.githubusercontent.com/junegunn/fzf/master/shell/key-bindings.zsh' nocompile pick'key-bindings.zsh' @zdharma-continuum/null

Downloading zdharma-continuum/null… (at label: dltest…)
Cloning into '/home/jan/.local/share/zinit/plugins/dltest'...
⠙ ███████████ OBJ: 100, PACK: 8/8, COMPR: 100%, REC: 100%
[patch-dl annex]: File key-bindings.zsh downloaded correctly

(the last line didn't show up before this fix, but I have not verified if this actually loads the right file and so on as I don't use fzf that much)

Types of changes

Checklist:

jankatins commented 1 year ago

The ubuntu cases fail with

Run brew install ncurses svn unzip zsh
/home/runner/work/_temp/c245a1ac-e773-41eb-bb97-2f6b316ec4[8](https://github.com/zdharma-continuum/zinit/actions/runs/3267870424/jobs/5373603132#step:4:9)1.sh: line 1: brew: command not found
Error: Process completed with exit code 127.

-> I have no clue, I thought brew would be used on mac?

Seems the zshelldoc test is also completely out of date, at least I didn't change so many places :-) https://github.com/zdharma-continuum/zinit/actions/runs/3267870423/jobs/5373603140

vladdoster commented 1 year ago

In the latest version of the Ubuntu GitHub runner, they don't include Homebrew in $PATH. I'll finish work in #380 to unblock you and get tests green again.

Homebrew on Linux is named Linuxbrew. You use same install script with the only caveat is that Linuxbrew does not support Casks.

jankatins commented 1 year ago

Might be faster to check this PR and then regenerate the zshelldoc test :-)

(the last time I couldn't get that part to work and in the end I manually edited the files to confirm to whatever the test wanted :-))

jankatins commented 1 year ago

So, I ran into this now myself:

zinit default-ice --quiet as'null' from"gh-r" lbin'!' lucid nocompile nocompletions
# Faster way to switch between clusters and namespaces in kubectl
zinit id-as'kubectx' lbin'!kubectx' bpick'kubectx*' dl'https://raw.githubusercontent.com/ahmetb/kubectx/master/completion/_kubectx.zsh -> _kubectx' for @ahmetb/kubectx
zinit id-as'kubens' lbin'!kubens' bpick'kubens*' dl'https://raw.githubusercontent.com/ahmetb/kubectx/master/completion/_kubens.zsh -> _kubens' for @ahmetb/kubectx

Without the change in this PR:

λ exec zsh

Downloading zdharma-continuum/zinit-annex-patch-dl…
Cloning into '/Users/jankatins/.local/share/zinit/plugins/zdharma-continuum---zinit-annex-patch-dl'...
⠙ ███████████ OBJ: 100, PACK: 9/9, COMPR: 100%, REC: 100%
Note: Compiling: z-a-patch-dl.plugin.zsh… OK.

Downloading ahmetb/kubectx… (at label: kubectx…)
(Requesting `kubectx_v0.9.4_darwin_x86_64.tar.gz'…)
#################################################################################################################################################################################################### 100.0%
ziextract: Unpacking the files from: `kubectx_v0.9.4_darwin_x86_64.tar.gz'…
ziextract: Successfully extracted and assigned +x chmod to the file: `kubectx'.
linkbin annex: Created the kubectx soft link and set +x on the kubectx binary

Downloading ahmetb/kubectx… (at label: kubens…)
(Requesting `kubens_v0.9.4_darwin_x86_64.tar.gz'…)
#################################################################################################################################################################################################### 100.0%
ziextract: Unpacking the files from: `kubens_v0.9.4_darwin_x86_64.tar.gz'…
ziextract: Successfully extracted and assigned +x chmod to the file: `kubens'.
linkbin annex: Created the kubens soft link and set +x on the kubens binary

With the patch:

λ  exec zsh

Downloading ahmetb/kubectx… (at label: kubectx…)
(Requesting `kubectx_v0.9.4_darwin_x86_64.tar.gz'…)
#################################################################################################################################################################################################### 100.0%
ziextract: Unpacking the files from: `kubectx_v0.9.4_darwin_x86_64.tar.gz'…
ziextract: Successfully extracted and assigned +x chmod to the file: `kubectx'.
[patch-dl annex]: File _kubectx downloaded correctly
linkbin annex: Created the kubectx soft link and set +x on the kubectx binary

Downloading ahmetb/kubectx… (at label: kubens…)
(Requesting `kubens_v0.9.4_darwin_x86_64.tar.gz'…)
#################################################################################################################################################################################################### 100.0%
ziextract: Unpacking the files from: `kubens_v0.9.4_darwin_x86_64.tar.gz'…
ziextract: Successfully extracted and assigned +x chmod to the file: `kubens'.
[patch-dl annex]: File _kubens downloaded correctly
linkbin annex: Created the kubens soft link and set +x on the kubens binary

Interestingly, I have found no way with other ices to actually get the completions getting recognized during installation: could it be that the completions are searched for before the dl ice runs (compile seems to also run beforehand!) ? In the end I ran zinit creinstall kubens && zinit creinstall kubectx once and it works now (seems the symlinks are enough to get this loaded).

seagle0128 commented 1 year ago

I can confirm this fix the issue. Please merge the PR. Thanks!

vladdoster commented 1 year ago

@jankatins you are on fire with bug fixes! Your contributions have fixed some subtle bugs, much thanks for taking the time to triage and fix them.

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 3.8.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: