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

Core module or optional dependencies #142

Closed
ColinHebert opened this issue Apr 13, 2012 · 8 comments
Closed

Core module or optional dependencies #142

ColinHebert opened this issue Apr 13, 2012 · 8 comments

Comments

@ColinHebert
Copy link
Contributor

I know we already talked about that module, and how hard it is to define what needs to be in the core, or not.

But I think that we need now to provide one "big module" so people don't have to worry about the order in which modules are loaded.
This kind of issue affects(ed?) in particular alias, environment, gnu-utility, completion and utility.

These modules must be loaded in a specific order:

  • gnu-utility depends on environment (to set $path, toward the gnu commands)
  • alias depends on gnu-utility (to load the gnu ls for colors)
  • completion depends on alias (to get $LS_COLORS for completion)
  • utility depends on completion (for some compdef calls)

These "dependencies" aren't mandatory so there is no need to 'force-load' the module, but if they are used, they must be declared in the right order.


I can see two solutions:

  1. Create a core module which loads every of these modules (plus some others) in the right order. This way new users won't be able to easily mess with the list of loaded modules.
  2. Create a new function which allows to declare an optional dependency, a bit like omodload does but this one wouldn't "force" the plugin to be loaded, but instead check if the plugin is declared in ':omz:load' omodule, and if it is, load the said module.

Both methods will avoid future troubles with the loading order of modules, but the first solution means that gnu-utils has to be a part of core (fortunately, there is an idiot-proof check which mean that if you don't really need this module it won't be loaded anyway) and the second one, besides of being a bit more complicated to implement, creates a new level of dependencies which can be a nighmare to handle.

There is actually a third solution (which is complementary to both solutions), which is specifying in the documentation which "optional modules" are supported.

@sorin-ionescu
Copy link
Owner

The best way to handle dependencies is to add a comment to the zshrc template which says, the order matters. As for non-default modules, it's best to mention in the README.md how a module should be loaded. For, gnu-utility, for example, one would mention that this module should be loaded after environment but before alias.

@ColinHebert
Copy link
Contributor Author

I'm really not sure that we should rely only on documentation, even if I agree on the fact that everyone should read it, too many people will just skip/skim it.

I understand that your answer to that would be "not my problem, it was written in the documentation", but I mean, at some point we need to be consistent between a flexible solution where we anticipate errors/mistakes from the user (typically the fool-proof check in gnu-utility and some other stuff quite similar in other modules) or a strict one based on readme files saying that if you don't respect the basic documentation you can only complain to yourself.

Actually, I'm really neutral on this choice, but I think we need to choose only one solution; using both seems to be more confusing than anything. I'm all for a complete documentation for each module that you have to read and understand before doing anything. But I don't mind either saying "okay, play with the application, we handle most scenarii so you don't have to worry about breaking things".

In this particular case, justing saying the order matters in the documentation doesn't match what we do in some other modules (other part of the code) where we manually check for errors in order to prevent user's mistakes. It's just a question of consistency through OMZ. (once again I don't really care which kind of solution is chosen, I just think that one should be chosen and everybody should stick to it, [almost] no matter what.)

@sheerun
Copy link

sheerun commented Jan 13, 2013

I don't think describing dependencies in README's is the most optimal solution. Why do you think it's the best solution? If it can be done automatically, then why not implement that?

Do you think of any way that could work? For example require 'environment' function or similar?

@sorin-ionescu
Copy link
Owner

It's pmodload 'environment' in Prezto, which is akin to the Zsh zmodload command.

@sheerun
Copy link

sheerun commented Jan 13, 2013

OK. So the order doesn't matter now. Good to know.

@sorin-ionescu
Copy link
Owner

The order does matter. A module that can work without other modules present will not show a warning or error but will just not enable functionality with missing dependencies.

@sheerun
Copy link

sheerun commented Jan 13, 2013

So pmodload 'environment' won't load enviroment module if it's not loaded earlier (listed in :prezto:load)?

Isn't it possible to process whole list passed to zstyle ':prezto:load' pmodule and only after that load these modules so the order won't actually matter?

I think there should be also clear distinction if pmodload requires dependency or makes it optional. Maybe split it to two functions: pmodload and pmodrequire or something like that.

@sorin-ionescu
Copy link
Owner

Play with the modules in zpreztorc. See if you can get any errors to show up.

krig added a commit to krig/prezto that referenced this issue Oct 24, 2015
* prompt/external/pure 0421252...dec0253 (26):
  > Update oh-my-zsh instructions
  > 1.2.0
  > Bump zsh-async to 1.0.0, prevents mixed stdout/stderr
  > Merge pull request sorin-ionescu#153 from sindresorhus/preprompt-update-fix
  > Merge pull request sorin-ionescu#149 from mafredri/pure-nitro
  > Close sorin-ionescu#147 PR: Preserve preprompt on Ctrl+L. Fixes sorin-ionescu#145
  > Remove cr from prompt_opts, fixes sorin-ionescu#127
  > Merge pull request sorin-ionescu#142 from zmwangx/string-length-fix
  > Merge pull request sorin-ionescu#144 from zmwangx/rename-prompt-to-preprompt
  > Default to 0 for git rev-list left and right. Fixes sorin-ionescu#137.
  > Close sorin-ionescu#140 PR: Add git arrows customization. Fixes sorin-ionescu#139.
  > 1.1.1
  > Disable prompt expansion for running command
  > Merge pull request sorin-ionescu#130 from zmwangx/rename-variables-for-readability
  > Use standard `[[ ]]` for conditional and add clarifications
  > fix: do the PURE_GIT_PULL check in the correct place
  > 1.1.0
  > Close sorin-ionescu#124 PR: Show hostname in terminal title if session is over ssh.
  > Merge pull request sorin-ionescu#125 from sindresorhus/dirty-check
  > import bug-fix release from zsh-async, fixes async job flushing
  > readme: faq clarificaitons
  > readme: add zpty error to faq with explanation and potential solutions
  > readme: update instructions for antigen and oh-my-zsh. remove incompatible async.plugin.zsh
  > use cd -q to prevent hooks from firing
  > prevent git status leakage when testing if dirty
  > fix paths that are split due to spaces in directory names
kodelint pushed a commit to kodelint/prezto that referenced this issue Nov 15, 2016
Add 'line' highlighter for the whole buffer
kodelint pushed a commit to kodelint/prezto that referenced this issue Nov 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants