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

Document better or detect the fact that .zimrc should not be used as it was sourced during shell startup #448

Open
ericbn opened this issue Dec 17, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@ericbn
Copy link
Member

ericbn commented Dec 17, 2021

Is your feature request related to a problem? Please describe.
When users define variables in their .zimrc file, the variables will be set after running zimfw with most commands*, giving users the wrong impression that they've done the right thing. But .zimrc is not sourced during the shell startup. So those variables will not be set when starting a new terminal.

Describe the solution you'd like
A nice but maybe too complex solution would be to detect this scenario and to print a warning to the user.

Or to source .zimrc in a "closed" way where everything defined there is forced to be local -- not sure if this is possible though.

Describe alternatives you've considered
Or maybe we should just document this better.

Another option would be to only allow zmodule calls in the .zimrc, and print a warning or error for anything else. The reason I don't like this option, and why I made the .zimrc a complete Zsh script, is because the intention is to allow users to still add any logic they want in there. There hasn't been a real practical use of the extended scripting beyond just the plain zmodule calls though.

Additional context
This has been a common misunderstanding from users, most recently by @Bananaman here.

*build, clean, clean-compiled, clean-dumpfile, compile, init, list, install, update, uninstall

@ericbn ericbn added the enhancement New feature or request label Dec 17, 2021
@Arcitec
Copy link

Arcitec commented Dec 18, 2021

Excellent idea for discussion!

The thing that initially fooled me was rc in .zimrc since that stands for "run commands" and normally refers to commands that will run at every startup. If it had been named .zimcfg or something, I wouldn't have thought it was an rc file.

The second thing that fooled me was that the .zimrc does get sourced at startup once, if you have modified its contents since the previous time you started a shell. This gave me the wrong impression that my variables in the .zimrc were being set at every startup.

That being said, it seems like the only things that should go in .zimrc are zmodule statements. Meaning that it's a list of what modules to build into zim's "init.zsh" script (as far as I understood it, that file gets rebuilt dynamically based on the zimrc).

If so, my proposal would be to scan the .zimrc contents and only accept lines that are comments, whitespace or zmodule. It would be great if anything else produced warnings, such as:

WARNING: The following lines should be moved from ~/.zimrc to ~/.zshrc:
export YSU_MODE=ALL

@ericbn
Copy link
Member Author

ericbn commented Dec 18, 2021

@Bananaman, great point on the rc in the filename! I should have considered changing the name when Zim 1.0 was released.

EDIT: Or maybe .zimrc can be interpreted as the "run commands for Zim", which is not incorrect, as that's the file that's read when you run zimfw...

@Arcitec
Copy link

Arcitec commented Dec 18, 2021

@ericbn I guess so, but it's not too late to deprecate it, like if .zimcfg exists, use it, else look for .zimrc if the decision to rename it feels right.

But the filename being a bit confusing isn't a major issue, as long as one of two things happens:

  1. Either warn users that they probably didn't intend to put export YSU_MODE=all in .zimrc.
  2. Or, perhaps source it at startup but try to do it in a "smart" way with a dummy zmodule function which does nothing during normal startups. Meaning that it literally just loads the variables (if any). This feels a bit dirty though and would slow down the precious startup time metric. :)

I saw your edits to the top post now, with the complexities, such as "what if someone uses complex logic such as if X, zmodule Y". Yeah, my proposal (number 1) would break their scripts in that case, if someone does that... I can see why this is hard to solve.

@Arcitec
Copy link

Arcitec commented Dec 18, 2021

I had two ideas...

  • If you decide to parse the .zimrc and look for illegal (non-zmodule, non-comment) lines, you could just add a special marker for people who use advanced .zimrc scripting on purpose. For example, let them write # advanced as the first line of their .zimrc, which could tell zimfw "don't analyze this file, the user knows what they're doing".
  • Perhaps another solution would be to add a command such as zimfw checkrc which scans the .zimrc and warns if anything weird exists in there. This manual command would be visible in zimfw help and on the website and might be enough to alert people that there's something "going on" with the zimrc format that they'll want to check/sanitize. Edit: Although this wouldn't protect the lazy/impatient users who never RTFM. Hehe.

@ericbn
Copy link
Member Author

ericbn commented Dec 18, 2021

An ideia for a Zim version 2.0 would be to allow users to define everything in their .zshrc file. I think the necessity to keep the module definitions and configurations as close to each other as possible makes perfect sense.

On the other hand, I like how we can easily detect if the .zimrc file changed, and also only run the zmodule commands when we need them (not taking up startup time on that).

@Arcitec
Copy link

Arcitec commented Dec 18, 2021

Hmm, I like the idea of zmodule and config being close together. Initially, my thought was something like this:

"Okay, the zmodule command is what loads the module from disk and sources the script. So if I want to configure a module, I have to put the export SETTING=value stuff before the zmodule line so that my config is set before the module script is sourced. And all of it has to be in my .zimrc."

It just wasn't clear enough (I am still unsure actually) what the .zimrc actually does. It seems like it's only being used as instructions for zimfw for compiling a single, pre-computed, "hidden" init.zsh file that refers to all of the user's modules. So it's kind of like a red herring: It looks like "the rc file that runs at every startup and loads all modules" but it's just a config file for zimfw.

The idea of merging it with the .zshrc is actually good, by the way. It would be cleaner that way. So the .zshrc would be "the" configuration place, and it would be divided into "load zimfw itself, then load modules and their settings, and finally the user's own regular zsh settings at the bottom of the file".

You could just move the "did .zimrc change?" to "did .zshrc change?". It won't be changing often enough that the false positives matter (i.e when zshrc changed but no zmodule lines were added/deleted). I think such a change wouldn't affect any users negatively. :)

Edit: And yeah moving everything to the .zshrc would mean that all zmodule lines are interpreted at every startup, which is the only negative downside, but hopefully those calls are very fast and won't hurt zim's legendary startup performance.

@ericbn
Copy link
Member Author

ericbn commented Dec 18, 2021

You're correct in your current ideas about .zimrc. The action that compiles the init.zsh from .zimrc is zimfw build (which is also internally called in other actions -- check zimfw help). The generated init.zsh is not so "hidden" though. You can even take a look at it, and it will be very clear what is going on in there: cat ${ZIM_HOME}/init.zsh.

And yeah, we're on the same page about the ideas for the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants