Suppress "omf install" warning?

[jonscottclark]

Testing out a new install using the updated theme/plugin syntax and hit upon a minor pain point when using it in conjunction with my dotfiles. (Beware, very stream-of-consciousness, apologies in advance)

I have an ideal workflow when setting up a new environment:

• Install fish via package manager
• Install oh-my-fish via curl
• Get my dotfiles from GitHub and run my bootstrapper script

In my bootstrapper script, I move the dotfiles from my repo to the user's home directory. One of those files is my config.fish which replaces the default config provided by oh-my-fish. At the end of my script, I reload the shell so that the changes take effect, and then I get a wall of red text re: using omf install to add the plugins and theme that are specified in my custom config.fish.

So I decided I'd add omf install to the end of the script before the shell gets reloaded so that all the plugins get added.

Unfortunately, that just installs what's currently set for $fish_theme and $fish_plugins in the current shell session (what ships by default: "robbyrussell" theme and "theme" plugin). Worse, if the shell wasn't reloaded after installing oh-my-fish, running omf install throws an error because of a missing argument ($fish_theme, etc. don't exist in the session).

So before I call omf install I source the config.fish file that just got copied so that the $fish_theme and $fish_plugins get updated. Works great.

But there's still that wall of red text :(

My PR-fu isn't the greatest, so I'll describe what I did to make the warnings disappear in my case: I made a fork of oh-my-fish to include a check on an option called fish_suppress_install_warning before notifying the user. In my custom config.fish in my dotfiles repo, I set that variable to true (globally). Now, after the file gets copied to my home directory during the bootstrapper script, I can source my config.fish without getting the warnings (getting the $fish_theme and $fish_plugins variables updated to contain my chosen theme/plugins), run omf install, and then reload the shell to load up the new theme, plugins, aliases, etc.

I realize this is an edge case and complete matter of preference, but what do you think about allowing a user to provide an option in a config.fish file to not see this new warning? If this is just silly, please let me know (still new to contributing). I can always just put a little message in my script saying "don't be afraid of the scary red text" and everything will still work as expected. Or maybe if the colour wasn't red I wouldn't have had such a bad reaction. Anyway, that's all I got, cheers! :smile:

tl;dr :cat: :thought_balloon: plz add option to suppress install warning. maybe dis?

[jonscottclark]

I guess another thing that stems from my efforts on this is that after changing config.fish, you have to start a new session to be able to install whatever theme or plugins you've put in your config.

So in the sentence in the README that states "Open your fish configuration file ~/.config/fish/config.fish and specify the theme and the plugins you want to use. And then run omf install on your terminal to install them." isn't entirely accurate, since omf install reads from the currently loaded $fish_plugins and $fish_theme variables, not from whatever's in your current config.fish.

Then, when the plugins are installed, another shell session has to be started for the theme and plugins to take effect. Should shell-reloading functionality be bundled with with omf install or would that violate separation of concerns and should be left to the user instead?

[bpinto]

What if we reload oh-my-fish before running any omf command? omf install, omf update...

I think the suppress warning option is good for a short term solution though.

[bpinto]

I guess if I had run my boxen setup I'd have noticed this problem, but I installed it manually instead. :(

[jonscottclark]

Are you talking about reloading config.fish as a whole? Because the Theme and Plugin statements are located there?

Or would there be some way to get Oh My Fish to read your config.fish and evaluate the Theme and Plugin statements to update the $fish_theme and $fish_plugins variables? Then, omf install would install whatever's in your config.fish, no matter whether you've reloaded your shell after editing it.

And then what about reloading the shell afterwards? The newly-installed plugins won't be available until the shell is reloaded. In my case, after my dotfiles get moved over, I'll be reloading my shell after my bootstrapper is done executing everything, and by default I wouldn't really expect a shell reload to be rolled into omf install.

If I were in your shoes I would give this some more thought (though I like the suppress option in the short term, as well). Maybe in the meantime make a note in the README about reloading your shell before and after an omf install (anybody who is customizing their shell should be fairly aware of this requirement, maybe a friendly reminder in the README would just clear up any confusion people might have when faced with the warnings).

Ideally we'd just partially re-read config.fish (avoiding execution of anything else in it, if that's even possible) to update the vars we need to do an install; maybe have a shell reload built into omf install (or update, or whatever else), and if so, allow users to provide an option to not reload afterwards, as some people might want to do it manually later (in a script, for instance).

[jonscottclark]

I'll try implementing what I suggest in the last paragraph and let know how it goes, maybe submit some PRs for you to check out. In the meantime let me know if you think of any other approaches.

[bpinto]

I thought about what you said, and I think you have a valid point, we shouldn't reload the framework.

At the same time, reading the configuration file would be interesting. And this wouldn't stop us being able to support remote repos (e.g. Plugin 'abc', github: 'user/abc')

[jonscottclark]

Well, I have it working (sorta), reading config.fish, looking for any lines that start with Plugin or Theme and calling those functions again with the values found as arguments to those lines; but I discovered a pitfall. If any user has platform specific configs which get loaded based on the uname (as I do), or any other external config which calls Plugin or Theme, it doesn't work. Nor do I think it will ever. Reloading the shell might be the only way to make sure the entirety of the user's config gets refreshed. Might just have to be an inconvenience that users need to live with and get used to. With adequate documentation about what's required to get your config into the current shell session it shouldn't prevent anybody from figuring out how to use the framework. I'll leave the issue open in case any more thoughts/discussion occurs.

[bpinto]

Could you help on the documentation? It seems like this is very clear in your mind.

Thanks for the thorough investigation though.

[jonscottclark]

Yeah, I would love to help. I'll submit PRs when ready and try to address everything I can.

[bpinto]

Fixed with #512