The mangled suffix breaks usage with antigen and zgen

[belak]

This was caused by dbeb163

Because of how antigen works, when it loads from the cache, at the point where it sources the plugin file, $1 is unfortunately set to apply, so all the gitstatus functions are loaded with the apply suffix with breaks quite a few things.

Additionally, on initial load with zgen, the following error pops up:

/Users/belak/.zgen/romkatv/gitstatus-master/gitstatus.plugin.zsh:typeset:58: not valid in this context: _gitstatus_plugin_dir/Users/belak/.zgen/romkatv/gitstatus-master/gitstatus.plugin.zsh

It seems like at that point, $1 is /Users/belak/.zgen/romkatv/gitstatus-master/gitstatus.plugin.zsh.

zplug works, but this is only because it uses shift so by the time the file is sourced, $1 is no longer set: https://github.com/zplug/zplug/blob/master/base/core/load.zsh#L62-L81

Sample usage with antigen:

[[ ! -d "$HOME/.antigen" ]] && git clone https://github.com/zsh-users/antigen.git "$HOME/.antigen"
source "$HOME/.antigen/antigen.zsh"
antigen bundle romkatv/gitstatus
antigen apply

Sample usage with zgen:

[[ ! -d "$HOME/.zgen" ]] && git clone https://github.com/tarjoilija/zgen.git "$HOME/.zgen"
source "${HOME}/.zgen/zgen.zsh"
if ! zgen saved; then
  zgen load romkatv/gitstatus
  zgen save
fi

Is there a different variable that could be used other than $1? It seems like the current setting is an easy way to silently break things in unexpected ways.

[romkatv]

Because of how antigen works, when it loads from the cache, at the point where it sources the plugin file, $1 is unfortunately set to apply [...].

That sounds like a bug.

Imagine if antigen exposed an API for running a command after compinit.

antigen run-after-compinit my-command

And imagine that it would invoke my-command with unspecified arguments. That would be a bug, right? It should invoke my-command and not my-command apply or anything like that.

The same logic applies to source. antigen should source without arguments. That is, instead of this:

source "$some_file"

It should do this:

set --
source "$some_file"

Is there a different variable that could be used other than $1? It seems like the current setting is an easy way to silently break things in unexpected ways.

Going back to run-after-compinit analogy, it's certainly possible to work around the underlying bug by never using positional arguments in my-command and instead using named global parameters. It is a workaround though. The right thing to do is to fix the bug in antigen.

[belak]

I understand where you're coming from, but the fact that this bug exists in multiple plugin managers that we know of (both zgen and antigen) means it's probably more widespread and would be easier to fix here.

I'll have to see if I can wrangle up the antigen and zgen maintainers, but I'm not optimistic as zgen hasn't been updated in over 2 years and antigen hasn't seen any code changes during that time period either.

[romkatv]

the fact that this bug exists in multiple plugin managers that we know of (both zgen and antigen) means it's probably more widespread

True. On the plus side, this bug doesn't exist in zplug and zinit.

and would be easier to fix here.

Here it wouldn't be a fix but a workaround. You are the first person to bring this up, so I suppose the number of users who would benefit from this workaround is small.

By the way, why do you care about antiques such as antigen and zgen? How did this issue come to your attention?

[belak]

I've been using antigen in my config for a while - the reason this didn't show up right away is because I don't make it a habit to run the command to update my zsh bundles, so I only noticed this on a new install. I'd imagine plenty of other people are the same way. Anyway, after my new install, my theme stopped showing the git prompt (because it's only enabled if gitstatus_start exists).

At least to me, when sourcing a file I don't expect it to handle arguments and I especially don't expect it to rename functions based on an argument passed to a function outside the scope of that file. It took me a long time to debug this (actually multiple weeks since it wasn't super high priority, just an annoyance) because that behavior was the last thing I would have expected.

I've never been a fan of zplug, as I don't like that it invents a new syntax on top of zsh's for loading plugins... if I can't get antigen to work, maybe I'll have to try zinit. I liked antigen because it was fairly minimal and no-nonsense and has been stable for quite a while. Maybe one day I'll have to come up with yet another alternative.

[romkatv]

I can add GITSTATUS_FUNCTION_NAME_SUFFIX and use it like this:

function gitstatus_start"${GITSTATUS_FUNCTION_NAME_SUFFIX-$1}"() {
  ...
}

Then you'll be able to set GITSTATUS_FUNCTION_NAME_SUFFIX="" in your config to explicitly disable name mangling. You'll probably be the only person to use this parameter.

Do you see any other workarounds that wouldn't break backward compatibility?

[belak]

No, that's alright - don't add a workaround just for my sake. You're right that antigen/zgen should probably be updated anyway. I appreciate the offer though.