zdharma-continuum / zinit

🌻 Flexible and fast ZSH plugin manager
MIT License
3.1k stars 128 forks source link

[bug]: atclone/mv/... are not run when installing plugins non-interactively #199

Closed jankatins closed 2 years ago

jankatins commented 2 years ago

Issue description

When I remove my whole plugin dir, the direnv hock is not run, resulting in an error. If I remove only the direnv directory, everything is fine:

λ  rm -rf .zinit/plugins/

# Opening a new tab
... lots of zinit install ...
Downloading direnv/direnv…
(Requesting `direnv.linux-amd64'…)
############################################################################################################################################################################################################ 100.0%
ziextract: Successfully extracted and assigned +x chmod to the file: `direnv.linux-amd64'.
bin-gem-node annex: Warning: The sbin'' ice (`direnv') didn't match any files
.zinit-load-plugin:source:110: no such file or directory: /home/jan/.zinit/plugins/direnv---direnv/zhook.zsh

When I just remove the direnv, it works:

λ  rm -rf .zinit/plugins/direnv---direnv

Downloading direnv/direnv…
(Requesting `direnv.linux-amd64'…)
############################################################################################################################################################################################################ 100.0%
ziextract: Successfully extracted and assigned +x chmod to the file: `direnv.linux-amd64'.
No files for compilation found.
Warning: ∞zinit-compile-plugin-hook hook returned with 1
bin-gem-node annex: Created the direnv shim and set +x on the direnv binary

I tried several way to work around this (e.g. with or without mv"direnv* -> direnv" or sbin"direnv* -> direnv", with dot in the mv or without), but none of them worked. Even running the direnv line directly after installing the sbin plugin had the same effect: if I remove th ewhol plugin dir, it fails to mv the direnv binary to direnv and does not run the atclone line.

zinit config

  source ~/.zinit/bin/zinit.zsh

  zinit light-mode for zdharma-continuum/zinit-annex-bin-gem-node

  # some oh-my-zsh niceties
  # OMZ plugins: https://github.com/ohmyzsh/ohmyzsh/tree/master/plugins
  zinit wait lucid for \
    atinit"ZINIT[COMPINIT_OPTS]=-C; zicompinit; zicdreplay"  \
          OMZL::key-bindings.zsh \
          OMZL::history.zsh \
          OMZP::command-not-found \
          OMZP::extract \
          OMZP::colored-man-pages \
          OMZL::completion.zsh \
          OMZL::termsupport.zsh \
          zdharma-continuum/fast-syntax-highlighting \
          paulirish/git-open \
          zsh-users/zsh-completions \
    pick"venv-lite.zsh" \
          jankatins/venv-lite.zsh \
    as"completion" \
          OMZP::docker/_docker \
    as"completion" \
          OMZP::docker-compose/_docker-compose \
    as"completion" \
          OMZP::pip/_pip \
    atload"!_zsh_autosuggest_start" \
          zsh-users/zsh-autosuggestions \
          zdharma-continuum/history-search-multi-word \
          zdharma-continuum/zsh-navigation-tools \
          zsh-users/zsh-syntax-highlighting \
          from"gh-r" sbin"starship" \
          atclone"./starship init zsh > init.zsh; ./starship completions zsh > _starship" \
          atpull"%atclone" src"init.zsh" \
          load starship/starship \
          from"gh-r" sbin"navi* -> navi" \
          load denisidoro/navi \
          from"gh-r" sbin"gitui* -> gitui" \
          load extrawurst/gitui \
          from"gh-r" sbin"**/delta -> delta" \
          load dandavison/delta \
          from'gh-r' sbin'fzf' \
          load junegunn/fzf \
          from"gh-r" sbin"**/bat -> bat" \
          load @sharkdp/bat \
          from'gh-r' sbin'bw' bpick"*.zip" \
          load bitwarden/cli \
          from"gh-r" bpick"*amd64.tar.gz" sbin"pet* -> pet" \
          load knqyf263/pet \
          from"gh-r" sbin"**/gh -> gh" \
          load cli/cli \
          from"gh-r" sbin"**/xh -> xh" \
          load ducaale/xh \
          as"program" sbin"**/git-fuzzy" \
          load bigH/git-fuzzy \
          from"gh-r" sbin"git-absorb-* -> git-absorb" \
          load tummychow/git-absorb \
          from"gh-r" sbin"logo-ls_*/logo-ls -> lls" \
          load Yash-Handa/logo-ls \
          from"gh-r" sbin"pup* -> pup" \
          load ericchiang/pup \
          from"gh-r" sbin"mcfly* -> mcfly" \
          load cantino/mcfly

  # I also tried several other versions, but none works when this is run with the whole zinit install
  zinit from"gh-r" mv"direnv.* -> direnv" sbin"direnv.* -> direnv" \
        atclone'mv direnv* direnv ; ./direnv hook zsh > zhook.zsh' atpull'%atclone' \
        src="zhook.zsh" for \
        direnv/direnv

zinit version or commit ID

bc1bedb59013eaec0b63abde6d6131bd64642eb0

zsh version

5.8.1

host info

CPUTYPE=x86_64 MACHTYPE=x86_64 XDG_SESSION_TYPE=wayland OSTYPE=linux-gnu 5.16.0-4-amd64 ID=debian

vladdoster commented 2 years ago

There is no bug here and is due to your direnv recipe.

The following example does:

  1. Install zinit and install direnv.
  2. Show direnv works
  3. Remove zinit plugin dir
  4. Start new zsh process which re-installs plugins
  5. Show direnv still works
Screen Shot 2022-03-14 at 06 32 53
vladdoster commented 2 years ago

I came up with the following recipe. Use with caution as I've never used direnv, but seems to work fine.


zi for \
    as"program" \
    atclone'./direnv hook zsh > zhook.zsh' \
    from"gh-r" \
    light-mode \
    mv"direnv* -> direnv" \
    src'zhook.zsh' \
  direnv/direnv
jankatins commented 2 years ago

nice trick with the exec zsh :-)

The recipe is with normal as"program" mode, not sbin :-(

I'm actually unsure how sbin interacts with the rest (does it replace/remove more than the path addition? Or some of the modifier do not work with sbin? If so: why do they work sometimes?). Maybe this is a sbin bug then?

vladdoster commented 2 years ago

sbin documentation

This isn't a bug, but IMO an unsuitable use case. A direnv recipe is documented and gives reasoning as to why it should be structured that particular way.

jankatins commented 2 years ago

why sbin: to have a clean short PATH.

I basically converted all my from"gh-r" as"program" mv"xx -> yy" to from"gh-r" sbin"xx -> yy" and it worked, but the direnv one didn't. To be honest, I have not yet a mental model how all of these ices work together, e.g. if sbin is an additional step (as I currently understand mv"xx -> yy" is to as"program") or if it replaces the whole "pipeline" (as the examples do not show as"program") and should be thought of as a kind of as"sbin". Or all of them are individual "actions" (and then: in which order?)? I just reread the sbin documentation and there it's also not clear to me, just that the above "search and replace" pattern works (or at least should work).

Given the differences depending if the install is run with all plugins or alone (alone it runs mv and atclone, with all, it doesn't), I thought this is a bug, at least somewhere.

vladdoster commented 2 years ago

Let's take a step back. Why are you deleting plugin dir? I'd be comfortable in saying that if you do that, you should expect errors because you are removing all your plugins.

jankatins commented 2 years ago

Mostly for two reasons:

jankatins commented 2 years ago

Another case, where it is not run: when using zinit update. Relevant logs:

*      Updating zinit...
Assuming --all is passed
Updating Zinit…
...
Updating direnv/direnv
(Requesting `direnv.darwin-amd64', version v2.31.0… Current version: v2.30.3.)
#################################################################################################################################################################################################################### 100.0%
ziextract: Successfully extracted and assigned +x chmod to the file: `direnv.darwin-amd64'.
...
_direnv_hook:2: no such file or directory: /Users/jankatins/.zinit/plugins/direnv---direnv/direnv

So in this case, I assume, the mv didn't happen and so the original hook script couldn't find the renamed binary.

In this case removing /Users/jankatins/.zinit/plugins/direnv---direnv/ and opening a new tab (+ waiting for the install to complete) fixed it.

(might be related to https://github.com/zdharma-continuum/zinit/issues/172 which is about atclone not run after updates)

vladdoster commented 2 years ago

I came up with the following recipe. Use with caution as I've never used direnv, but seems to work fine.

zi for \
    as"program" \
    atclone'./direnv hook zsh > zhook.zsh' \
    from"gh-r" \
    light-mode \
    mv"direnv* -> direnv" \
    src'zhook.zsh' \
  direnv/direnv

This is due to how you install direnv. The recipe I offered (referenced above) successfully updates. Disregard lines starting with gh-r as they are irrelevant.

Screen Shot 2022-03-28 at 16 27 58
vladdoster commented 2 years ago

Additionally, the issue isn't related to mv. The hooks direnv sets for precmd and precwd are never removed. It's commented somewhere in zinit source code that they aren't removed.

Screen Shot 2022-03-28 at 21 38 17
jankatins commented 2 years ago

(Summary at the end of this comment...)

I tried to debug the initial problem (direnv atclone/mv/... ices are not executed) and for whatever reason (I use the update/rerun in a bootstrap script), I started comparing interactive usage with zsh -c zinit runs. After some tracing, diffing and so on, I ended up with a difference when running the following commands:

(the source ~/.zinit/bin/zinit-install.zsh; is in there because I added the printed lines and didn't want to remember when I had to source that file in my local shell)

The interesting part is in these lines:

https://github.com/zdharma-continuum/zinit/blob/6f039dfaee1915f0d0d00b9c26636ee7011959f1/zinit-install.zsh#L458-L473

The reply was empty (at least print -l $reply only printed an empty line) in non-interactive usages, but contained stuff on interactive calls. A few lines further, the "non exclamation mark" version worked in all cases)

If I add a the following debug prints:

        echo -e "\n\n### ZINIT_EXTS2:"
        print -l $ZINIT_EXTS2
        echo -e "\n\n### ZINIT_EXTS:"
        print -l $ZINIT_EXTS
        echo -e "\n\n### setop options:"
        setopt
        echo -e "\n\n### Key to lookup exclamation mark hooks:"
        echo "(I)zinit hook:\\\!atclone-pre <->"
full zinit-install.zsh diff

```diff diff --git a/zinit-install.zsh b/zinit-install.zsh index 7993ec85..eaab451f 100644 --- a/zinit-install.zsh +++ b/zinit-install.zsh @@ -451,6 +451,15 @@ builtin source "${ZINIT[BIN_DIR]}/zinit-side.zsh" || { } } + echo -e "\n\n### ZINIT_EXTS2:" + print -l $ZINIT_EXTS2 + echo -e "\n\n### ZINIT_EXTS:" + print -l $ZINIT_EXTS + echo -e "\n\n### setop options:" + setopt + echo -e "\n\n### Key to lookup exclamation mark hooks:" + echo "(I)zinit hook:\\\!atclone-pre <->" + if [[ $update != -u ]] { hook_rc=0 # Store ices at clone of a plugin @@ -460,6 +469,8 @@ builtin source "${ZINIT[BIN_DIR]}/zinit-side.zsh" || { ${(on)ZINIT_EXTS[(I)z-annex hook:\\\!atclone-<-> <->]} ${(on)ZINIT_EXTS2[(I)zinit hook:\\\!atclone-post <->]} ) + echo -e "\n\n### exclamation mark reply:" + print -l $reply for key in "${reply[@]}"; do arr=( "${(Q)${(z@)ZINIT_EXTS[$key]:-$ZINIT_EXTS2[$key]}[@]}" ) # Functions calls @@ -478,6 +489,8 @@ builtin source "${ZINIT[BIN_DIR]}/zinit-side.zsh" || { ${(on)ZINIT_EXTS[(I)z-annex hook:atclone-<-> <->]} ${(on)ZINIT_EXTS2[(I)zinit hook:atclone-post <->]} ) + echo -e "\n\n### without exclamation mark reply:" + print -l $reply for key in "${reply[@]}"; do arr=( "${(Q)${(z@)ZINIT_EXTS[$key]:-$ZINIT_EXTS2[$key]}[@]}" ) "${arr[5]}" plugin "$user" "$plugin" "$id_as" "$local_path" "${${key##(zinit|z-annex) hook:}%% <->}" ```

I get:

I've no clue why in one case it returns some hooks and in some not. But when I change the three lines to only contain two backspaces (hook:\\\!atclone-pre -> hook:\\!atclone-pre), it's now the reverse: the zsh -c version is ok and the interactive/.zshrc version fails.

            reply=(
                ${(on)ZINIT_EXTS2[(I)zinit hook:\\!atclone-pre <->]}
                ${(on)ZINIT_EXTS[(I)z-annex hook:\\!atclone-<-> <->]}
                ${(on)ZINIT_EXTS2[(I)zinit hook:\\!atclone-post <->]}
            )
            echo -e "\n\n### exclamation mark reply:"
            print -l $reply

My interpretation is that zsh selects array elements with exclamation marks/backspaces in the key differently depending on interactive mode or not?

Regarding removing the whole plugins dir (rm -rf ~/.zinit/plugins or all subfolders in there: rm -rf ~/.zinit/plugins/*):

My interpretation is that ZINIT_EXTS2 is not filled when the plugin dir is not there and therefore all hooks are not available, not even the internal hooks for mv and so on. It also seems that ZINIT_EXTS2 is never updated after the first initialization.

So to sum up:

vladdoster commented 2 years ago

You're breaking zinit when you delete plugins like you are trying. Have a look in a plugins ._zinit dir.

If you aren't using zinit to properly remove plugins, you are pulling the rug out from under it and it doesn't clear things.

What's wrong with using zinit delete --all?

vladdoster commented 2 years ago

Can you confirm you have no zshrc config in place other than zinit? Please confirm you get the same results with minimal config.

https://zsh.sourceforge.io/Doc/Release/Functions.html#Hook-Functions

vladdoster commented 2 years ago

This also really doesn't pertain to the issue anymore and getting into the weeds.

I've stated the crux of the direnv issue and that it has been documented in the source code. The direnv hooks are not removed.

jankatins commented 2 years ago

Why rm instead of zinit: a) because it was convinient (I knew about the folder but not about zinit delete(and it's tab completion) and it seemed to work until it didn't without any outward error during install) and b) because my assumption was that only the plugin dir contains state of the plugins and so removing the dir(s) should not alter other zinit calls (e.g. mv ice is a core thingy, not a plugin). This assumptions is clearly false and I will try to see if I can figure out why and if there is any easy change to make it more resilient against careless user like me :-)

In any case, I tried the zinit delete way:

[11:22:55] λ  zinit delete direnv/direnv -y ; zsh -c 'source ~/.zinit/bin/zinit.zsh ; source ~/.zinit/bin/zinit-install.zsh; zinit from"gh-r" mv"direnv* -> direnv" for direnv/direnv'
No such (plugin or snippet): direnv/direnv.

Downloading direnv/direnv…
(Requesting `direnv.darwin-amd64'…)
#################################################################################################################################################################################################################### 100.0%
ziextract: Successfully extracted and assigned +x chmod to the file: `direnv.darwin-amd64'.

### ZINIT_EXTS2:
20 z-annex-data: make'' hook:!atclone-post ∞zinit-make-hook '' ''
11 z-annex-data: extract hook:atpull-post ∞zinit-extract-hook '' ''
5 z-annex-data: mv'' hook:no-e-!atpull-pre ∞zinit-mv-hook '' ''
2 z-annex-data: ICE[reset] hook:e-!atpull-post ∞zinit-reset-hook '' ''
21 z-annex-data: extract hook:!atclone-post ∞zinit-extract-hook '' ''
12 z-annex-data: compile-plugin hook:atpull-post ∞zinit-compile-plugin-hook '' ''
6 z-annex-data: cp'' hook:no-e-!atpull-pre ∞zinit-cp-hook '' ''
3 z-annex-data: atpull'!' hook:e-!atpull-post ∞zinit-atpull-e-hook '' ''
7 z-annex-data: compile-plugin hook:no-e-!atpull-pre ∞zinit-compile-plugin-hook '' ''
14 z-annex-data: make'!!' hook:!atclone-pre ∞zinit-make-ee-hook '' ''
15 z-annex-data: mv'' hook:!atclone-pre ∞zinit-mv-hook '' ''
10 z-annex-data: make'' hook:no-e-!atpull-post ∞zinit-make-hook '' ''
16 z-annex-data: cp'' hook:!atclone-pre ∞zinit-cp-hook '' ''
1 z-annex-data: -r/--reset hook:e-!atpull-pre ∞zinit-reset-hook '' ''
17 z-annex-data: compile-plugin hook:!atclone-pre ∞zinit-compile-plugin-hook '' ''
13 z-annex-data: ps-on-update hook:%atpull-post ∞zinit-ps-on-update-hook '' ''
18 z-annex-data: make'!' hook:!atclone-post ∞zinit-make-e-hook '' ''
8 z-annex-data: make'!' hook:no-e-!atpull-post ∞zinit-make-e-hook '' ''
22
19 z-annex-data: atclone hook:!atclone-post ∞zinit-atclone-hook '' ''
9 z-annex-data: atpull hook:no-e-!atpull-post ∞zinit-atpull-hook '' ''
22 z-annex-data: compile-plugin hook:atclone-post ∞zinit-compile-plugin-hook '' ''
4 z-annex-data: make'!!' hook:no-e-!atpull-pre ∞zinit-make-ee-hook '' ''

### ZINIT_EXTS:

### setop options:
extendedglob
localoptions
localpatterns
localtraps
rcquotes
noshortloops
warncreateglobal

### Key to lookup exclamation mark hooks:
(I)zinit hook:\!atclone-pre <->

### exclamation mark reply:

### without exclamation mark reply:
zinit hook:atclone-post 22
_direnv_hook:2: no such file or directory: /Users/jankatins/.zinit/plugins/direnv---direnv/direnv

-> the mv ice is not applied as the exclamation mark reply is empty.

As far as I know, this command should also error on your computer (e.g. you will end up with ~/.zinit/plugins/direnv---direnv/direnv.darwin-amd64instead of ~/.zinit/plugins/direnv---direnv/direnv) as, as far as I understand it, zsh -c should start with no ~/.zshrc config.

(I'm totally fine with the direnv hooks not getting removed and that it's my responsibility to "unload it". My only problem here is that my (non-interactive) bootstrap/update script does not work because that seems to be the original problem: when my update installed a new direnv, that failed and I went ahead and deleted the plugin dir to get into a known state and that just got me into more trouble; If non-interactive zinit calls would work, I will be able to adjust to zinit delete if I actually would need it, and be fine)

vladdoster commented 2 years ago

Are you sure zsh -c does what you think it does. I glanced at the man page which reads as:

-c    Take the first argument as a command to execute, rather than reading commands
      from a script or standard input.  If any further arguments are given, the first
      one is assigned to $0, rather than being used as a positional parameter.

-i    Force shell to be interactive.  It is still possible to specify a script to
      execute.

and

-f, --no-rcs

The system-wide zshenv file (usually /etc/zshenv or /etc/zsh/zshenv or /usr/local/lib/zsh/zshenv) is always read, the only way to avoid this is to edit the zsh binary or patch the source.

jankatins commented 2 years ago

I never changed any system zsh config, so whatever is in there is there from the zsh install. Running with the options you suggest still results in no mv ice applied:

[12:12:42] [1] ✗  zsh -i -c -f --no-rcs 'source ~/.zinit/bin/zinit.zsh ; source ~/.zinit/bin/zinit-install.zsh; zinit delete direnv/direnv -y ; zinit from"gh-r" mv"direnv* -> direnv" for direnv/direnv'
No such (plugin or snippet): direnv/direnv.

Downloading direnv/direnv…
(Requesting `direnv.darwin-amd64'…)
#################################################################################################################################################################################################################### 100.0%
ziextract: Successfully extracted and assigned +x chmod to the file: `direnv.darwin-amd64'.

### ZINIT_EXTS2:
20 z-annex-data: make'' hook:!atclone-post ∞zinit-make-hook '' ''
11 z-annex-data: extract hook:atpull-post ∞zinit-extract-hook '' ''
5 z-annex-data: mv'' hook:no-e-!atpull-pre ∞zinit-mv-hook '' ''
2 z-annex-data: ICE[reset] hook:e-!atpull-post ∞zinit-reset-hook '' ''
21 z-annex-data: extract hook:!atclone-post ∞zinit-extract-hook '' ''
12 z-annex-data: compile-plugin hook:atpull-post ∞zinit-compile-plugin-hook '' ''
6 z-annex-data: cp'' hook:no-e-!atpull-pre ∞zinit-cp-hook '' ''
3 z-annex-data: atpull'!' hook:e-!atpull-post ∞zinit-atpull-e-hook '' ''
7 z-annex-data: compile-plugin hook:no-e-!atpull-pre ∞zinit-compile-plugin-hook '' ''
14 z-annex-data: make'!!' hook:!atclone-pre ∞zinit-make-ee-hook '' ''
15 z-annex-data: mv'' hook:!atclone-pre ∞zinit-mv-hook '' ''
10 z-annex-data: make'' hook:no-e-!atpull-post ∞zinit-make-hook '' ''
16 z-annex-data: cp'' hook:!atclone-pre ∞zinit-cp-hook '' ''
1 z-annex-data: -r/--reset hook:e-!atpull-pre ∞zinit-reset-hook '' ''
17 z-annex-data: compile-plugin hook:!atclone-pre ∞zinit-compile-plugin-hook '' ''
13 z-annex-data: ps-on-update hook:%atpull-post ∞zinit-ps-on-update-hook '' ''
18 z-annex-data: make'!' hook:!atclone-post ∞zinit-make-e-hook '' ''
8 z-annex-data: make'!' hook:no-e-!atpull-post ∞zinit-make-e-hook '' ''
22
19 z-annex-data: atclone hook:!atclone-post ∞zinit-atclone-hook '' ''
9 z-annex-data: atpull hook:no-e-!atpull-post ∞zinit-atpull-hook '' ''
22 z-annex-data: compile-plugin hook:atclone-post ∞zinit-compile-plugin-hook '' ''
4 z-annex-data: make'!!' hook:no-e-!atpull-pre ∞zinit-make-ee-hook '' ''

### ZINIT_EXTS:

### setop options:
extendedglob
interactive
localoptions
localpatterns
localtraps
rcquotes
noshortloops
warncreateglobal

### Key to lookup exclamation mark hooks:
(I)zinit hook:\!atclone-pre <->

### exclamation mark reply:

### without exclamation mark reply:
zinit hook:atclone-post 22

[12:13:42] λ  ls -la ~/.zinit/plugins/direnv---direnv/direnv*
-rwxr-xr-x  1 jankatins  staff  8811104 Apr  7 12:13 /Users/jankatins/.zinit/plugins/direnv---direnv/direnv.darwin-amd64

I also ran it on my debian system in addition to the above mac/brew zsh install, same result: the mv doesn't happen:

[12:22:55] λ  ls -la ~/.zinit/plugins/direnv---direnv/direnv*
-rwxr-xr-x 1 jan jan 8926585 Apr  7 12:20 /home/jan/.zinit/plugins/direnv---direnv/direnv.linux-amd64
vladdoster commented 2 years ago

-f and --no-rcs are the same thing and I wasn't implying you should use all three. Just that from initial look at docs, -c might not be the correct flag.

vladdoster commented 2 years ago

Seems to be doing the check according to the docs

Screen Shot 2022-04-07 at 07 50 00

vladdoster commented 2 years ago

Also, seems this is already reported and has a solution in #129.

jankatins commented 2 years ago

129 seems to have a similar problem, but it is not based on non-interactivity. Looks more like the "delete the plugin dir" problem which we tried to background in this issue.

Anyway, just for my curiosity:

zsh -i -c --no-rcs 'source ~/.zinit/bin/zinit.zsh ; source ~/.zinit/bin/zinit-install.zsh; zinit delete direnv/direnv -y ; zinit from"gh-r" mv"direnv* -> direnv" for direnv/direnv'
ls -la ~/.zinit/plugins/direnv---direnv/direnv*

Is in your case showing ~/.zinit/plugins/direnv---direnv/direnv or ~/.zinit/plugins/direnv---direnv/direnv.<os>_<arch> (assuming you have your zinit plugin dir in that place...)?

I did some more digging and I have another clue: after staring at the printed output of ZINIT_EXTS2 I finally found a difference:

        for key value in ${(kv)ZINIT_EXTS2}; do
            echo "$key -> $value"
        done

When run interactively, it reads

[...]
zinit hook:\!atclone-pre 14 -> 14 z-annex-data: make\'\!\!\' hook:\!atclone-pre ∞zinit-make-ee-hook '' ''
zinit hook:\!atclone-pre 15 -> 15 z-annex-data: mv\'\' hook:\!atclone-pre ∞zinit-mv-hook '' ''

when run non-interactively

zinit hook:!atclone-pre 14 -> 14 z-annex-data: make\'!!\' hook:!atclone-pre ∞zinit-make-ee-hook '' ''
zinit hook:!atclone-pre 15 -> 15 z-annex-data: mv\'\' hook:!atclone-pre ∞zinit-mv-hook '' ''

-> the exclamation marks are quoted or not.

I'm unsure hwo this can be, given the registering of hooks is done via

@zinit-register-hook() {
    local name="$1" type="$2" handler="$3" icemods="$4" key="zinit ${(q)2}"
    ZINIT_EXTS2[seqno]=$(( ${ZINIT_EXTS2[seqno]:-0} + 1 ))
    ZINIT_EXTS2[$key${${(M)type#hook:}:+ ${ZINIT_EXTS2[seqno]}}]="${ZINIT_EXTS2[seqno]} z-annex-data: ${(q)name} ${(q)type} ${(q)handler} '' ${(q)icemods}"
    ZINIT_EXTS2[ice-mods]="${ZINIT_EXTS2[ice-mods]}${icemods:+|}$icemods"
}

so the key should be quoted.

I could reproduce this via:

λ  cat excl-test
test=x\!y ; echo "${(q)test}"  

λ  source excl-test
x\!y

λ  zsh -c 'source excl-test'
x!y

When running this line included in my .zshrc I also get the right x\!y output.

[edit]I'm unsure what's going on here, but it seems to be the source of the problem: the exclamation mark hooks are in the array under the "wrong" name and therefore it's not found when running it in non-interactive mode :-([/] See next comment

jankatins commented 2 years ago

It seems this is because history expansion is only happening in interactive mode. A workaround would be to use local histchars="" in @zinit-register-hook() before any usage of ${(q)...} and then go through the codebase and replace any \\\! with \!. Or do the lookups via variables and the same ${(q)...} usage.

That should then enable using zinit in scripts so one can run update via zsh -c "source zinit.zsh && zinit update --all" in a script.

@vladdoster Would you take a PR for one of these ideas?

vladdoster commented 2 years ago

@jankatins

Don't feel like you need to ask permission to open a PR, as all are welcome to contribute.

If you hit any snags, open a PR with current work, and we can work through them.

vladdoster commented 2 years ago

@jankatins Any update on this?

jankatins commented 2 years ago

@vladdoster Have a look at https://github.com/zdharma-continuum/zinit/pull/227

vladdoster commented 2 years ago

Updated title prefix and removed triage label for cohesiveness. Please don't change.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 3.8.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: