Repository: cantino/mcfly (language: rust)

Issue:
Use `source <(mcfly init *sh)` instead of `eval "$(mcfly init *sh)"`
Really enjoying this project, thank you 💕

As mentioned in #219 and #239, there is unexpected behaviour when re-running `.zshrc`. As @dasebasto [pointed out](https://github.com/cantino/mcfly/issues/219#issuecomment-1017339164), this is because of:

```zsh
if [[ "$__MCFLY_LOADED" == "loaded" ]]; then
  return 0
fi
__MCFLY_LOADED="loaded"
```

When `return 0` is `eval`'d it returns from the _whole_ `.zshrc` script, thus failing to run any further commands such as binding `bindkey '^R' mcfly-history-widget`, but more importantly failing to run the rest of `.zshrc` without warning.

Of course I totally agree that `source ~/.zshrc` is not _best_ practice. But it is _a_ practice. As such, it should not be unexpectedly broken without warning.

The quickest fix for this is just to update the README to suggest loading `mcfly` with `source <(mcfly init zsh)`. Another idea is to add an intermediate step, for example this is how [starship](https://github.com/starship/starship) does it:

```sh
# Show contents
% starship init zsh
source <(/usr/bin/starship init zsh --print-full-init)
# Run contents
% eval "$(starship init zsh)"
🚀
```
Stop working after sourcing ~/.zshrc
MacOS 11.6.2
iTerm 3.4.14
zsh 5.8 (with only git plugin)
at the end of .zshrc I have `eval "$(mcfly init zsh)"`

I also have fig.io installed

It works just after starting iTerm2, in default and newly created tabs

But after `.~/.zshrc` ^r is binded to standard back search



Discussion / hints:
Thank you for explaining this, I wasn't aware of that behavior of `return 0`. Should we just wrap the entire script in `if ! [[ "$__MCFLY_LOADED" == "loaded" ]]; then` ?
I think the script should remain unchanged, it's a good script. And anyway, it doesn't help you having to keep worrying about what you write there. Really the problem is the `eval "$(mcfly init zsh)"` in the README. Just updating the README with `source <(mcfly init zsh)` is enough.

But if you were up for it, the perfect solution would be what I suggested about an intermediate step. So that `mcfly init zsh` just returned `source <(mcfly init zsh --print-full-init)`. And then `mcfly init zsh --print-full-init` was the command that actually output the script. That way, all existing users get to benefit from the new approach without having to lift a finger 🤓
Any chance you want to make this change? ;) Otherwise, I can soon.

Edit: and thanks for the clear explanation!
Hey @andgineer, I'm not sure why it wouldn't be working.
Something in your .zshrc is probably binding ^r to standard or custom history search.

I had the same issue.
The culprit line in my .zshrc was:
`source $ZSH/oh-my-zsh.sh`

The bug only occurs when you source .zshrc more than once, because `$(mcfly init zsh)` contains this:

```
# Avoid loading this file more than once
if [[ "$__MCFLY_LOADED" == "loaded" ]]; then
  return 0
fi
__MCFLY_LOADED="loaded"
[...]
  bindkey '^R' mcfly-history-widget
```

Quick fix for me was to add
`bindkey '^R' mcfly-history-widget` *before* `eval "$(mcfly init zsh)` in my .zshrc


I believe the current behavior of `eval "$(mcfly init zsh)` should be changed. It should:
- always bind ^R
- not contain return statements that have the nasty side effect of skipping all the following lines in .zshrc
Thank you!
In fact it only help when I placed `bindkey` AFTER `source $ZSH/oh-my-zsh.sh`
I have the same problem (also with oh-my-zsh). I have tried putting `bindkey` before and after sourcing `oh-my-zsh`, but without luck. ctrl-r always get rebound back to the default search.

edit: I narrowed it down to the [zsh-vi-mode plugin](https://github.com/jeffreytse/zsh-vi-mode), probably because [it also binds ^R](https://github.com/jeffreytse/zsh-vi-mode/blob/master/zsh-vi-mode.zsh#L3288). After removing that, mcfly works as expected.
As mentioned in #254, a quick fix for this is to, instead of `eval "$(mcfly init zsh)"`, just use `source <(mcfly init zsh)`.

Fix patch:
diff --git a/src/init.rs b/src/init.rs
index c554ed5d..1e5c691e 100644
--- a/src/init.rs
+++ b/src/init.rs
@@ -2,14 +2,27 @@ use super::settings::InitMode;
 
 pub struct Init {}
 
+// Intermediating a `source <(...)` step is important so that the actual init
+// scripts don't have too much power in `.bashrc` or `.zshrc` when `eval()`'d. For
+// instance eval'ing `return 0` returns from the _entire_ .bashrc/.zshrc script,
+// not just the mcfly init script.
+//
+// Perhaps just asking users to directly `source <(mcfly init bash --print_full_init)`
+// is better, but we now also need to be backward compatible for those that already
+// have `eval "$(mcfly init bash)"` in their config.
+//
+// See: https://github.com/cantino/mcfly/issues/254
+const BASH_SOURCE_STANZA: &str = "source <(mcfly init bash --print_full_init)";
+const ZSH_SOURCE_STANZA: &str = "source <(mcfly init zsh --print_full_init)";
+
 impl Init {
-    pub fn new(init_mode: &InitMode) -> Self {
+    pub fn new(init_mode: &InitMode, is_print_full_init: bool) -> Self {
         match init_mode {
             InitMode::Bash => {
-                Init::init_bash();
+                Init::init_bash(is_print_full_init);
             }
             InitMode::Zsh => {
-                Init::init_zsh();
+                Init::init_zsh(is_print_full_init);
             }
             InitMode::Fish => {
                 Init::init_fish();
@@ -17,13 +30,21 @@ impl Init {
         }
         Self {}
     }
-    pub fn init_bash() {
-        let script = include_str!("../mcfly.bash");
-        print!("{}", script);
+    pub fn init_bash(is_print_full_init: bool) {
+        if is_print_full_init {
+            let script = include_str!("../mcfly.bash");
+            print!("{}", script);
+        } else {
+            print!("{}", BASH_SOURCE_STANZA);
+        }
     }
-    pub fn init_zsh() {
-        let script = include_str!("../mcfly.zsh");
-        print!("{}", script);
+    pub fn init_zsh(is_print_full_init: bool) {
+        if is_print_full_init {
+            let script = include_str!("../mcfly.zsh");
+            print!("{}", script);
+        } else {
+            print!("{}", ZSH_SOURCE_STANZA);
+        }
     }
     pub fn init_fish() {
         let script = include_str!("../mcfly.fish");
diff --git a/src/main.rs b/src/main.rs
index 5d07bc9e..2a0787c0 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -100,7 +100,7 @@ fn handle_move(settings: &Settings) {
 }
 
 fn handle_init(settings: &Settings) {
-    Init::new(&settings.init_mode);
+    Init::new(&settings.init_mode, settings.print_full_init);
 }
 
 fn main() {
diff --git a/src/settings.rs b/src/settings.rs
index 316af366..c424be2b 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -87,6 +87,7 @@ pub struct Settings {
     pub interface_view: InterfaceView,
     pub result_sort: ResultSort,
     pub disable_menu: bool,
+    pub print_full_init: bool,
 }
 
 impl Default for Settings {
@@ -116,6 +117,7 @@ impl Default for Settings {
             interface_view: InterfaceView::Top,
             result_sort: ResultSort::Rank,
             disable_menu: false,
+            print_full_init: false,
         }
     }
 }
@@ -243,6 +245,9 @@ impl Settings {
                     .help("Shell to init — one of bash, zsh, or fish")
                     .possible_values(&["bash", "zsh", "fish"])
                     .required(true))
+                .arg(Arg::with_name("print_full_init")
+                    .long("print_full_init")
+                    .help("Output full init code as opposed to just `source <(...)`"))
             )
             .get_matches();
 
@@ -460,6 +465,7 @@ impl Settings {
 
             ("init", Some(init_matches)) => {
                 settings.mode = Mode::Init;
+                settings.print_full_init = init_matches.is_present("print_full_init");
                 match init_matches.value_of("shell").unwrap() {
                     "bash" => {
                         settings.init_mode = InitMode::Bash;


Test patch: