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

Zsh #81

Merged
merged 16 commits into from
Jun 28, 2020
Merged

Zsh #81

merged 16 commits into from
Jun 28, 2020

Conversation

cantino
Copy link
Owner

@cantino cantino commented May 10, 2020

Initial work on Zsh support. I'm currently testing it locally.

Fixes #64. Fixes #3.

This was referenced May 10, 2020
Closed
@Flat
Copy link

Flat commented May 11, 2020

From my brief usage it seems to function fine. There does seem to be some unneeded command history after ctrl+r to use mcfly and after selecting a command such as:

~ on  mcfly
❯ #mcfly:

~ on  mcfly
❯  mcfly search

It would be better if these were hidden from the terminal. Not sure if this is intentional.

@cantino
Copy link
Owner Author

cantino commented May 11, 2020 via email

@Flat
Copy link

Flat commented May 11, 2020

You can find my full .zshrc here (with fzf replaced with mcfly of course)

Relevant? Zsh history ops:

setopt extended_history
setopt hist_expire_dups_first
setopt hist_ignore_dups
setopt hist_ignore_space
setopt hist_verify
setopt inc_append_history
setopt share_history 

@Haprog
Copy link

Haprog commented May 11, 2020

@cantino Thank you! This is awesome. I just installed this and briefly tested. I'm running zsh with oh-my-zsh on Pop!_OS (Ubuntu based Linux distro). I tried few searches and ran commands. Seems to work nicely. Didn't see any issues so far.

I didn't get any extra lines in history files or terminal (those that @Flat mentioned).

Ps. I didn't even remember what is mcfly since I never tried it before. I had just subscribed to notifications on the issue about zsh support and remembered that it was something I wanted to have but couldn't before. This is a very nice improvement to my workflow as I often use Ctrl+R search and the default one had pretty bad UX.

mcfly.zsh Show resolved Hide resolved
mcfly.zsh Show resolved Hide resolved
mcfly.zsh Show resolved Hide resolved
mcfly.zsh Show resolved Hide resolved
@Haprog
Copy link

Haprog commented May 12, 2020

I'm not sure what happened. I'm very sure I didn't get those extra lines when I tried this first time but noticed now (after reboot etc.) that now I get those extra lines showing in my terminal after using Ctrl+R:

#mcfly:
 mcfly search

@cantino
Copy link
Owner Author

cantino commented May 12, 2020 via email

@Flat
Copy link

Flat commented May 12, 2020

For me they seem to just be in the terminal buffer. Not in .zsh_history.

@cantino
Copy link
Owner Author

cantino commented May 13, 2020

That's the same behavior as bash. I haven't found a good way to prevent them, although @infokiller's suggestion might work for zsh.

@cantino
Copy link
Owner Author

cantino commented May 16, 2020

If anyone wants to test it again, I've worked with @infokiller's feedback and think I've removed the #mcfly: ... mcfly search nonsense in the command history.

@Haprog
Copy link

Haprog commented May 18, 2020

@cantino I just updated to the latest version in this branch (commit 8eecd8d) and I don't get the #mcfly: ... mcfly search in terminal anymore.

Otherwise this seems to work, but now arrow keys don't work in mcfly anymore to navigate up or down the suggestions list. Esc, Tab and Enter at least work as they should (can edit or run the first suggestion easily).

Actually while writing this I just noticed that PageUp and PageDown keys work to navigate the suggestions up/down, but I would much prefer to use arrow keys for it instead. I'm pretty sure arrow keys worked for this when I was using the initial version of this (commit 5a5784a)

@cantino
Copy link
Owner Author

cantino commented May 18, 2020

Thanks @Haprog, yes, that was definitely working before. Darn, I wonder if I can make zsh not consume those.

@cantino
Copy link
Owner Author

cantino commented May 19, 2020

Hey @Haprog, are you trying the current version? Arrow keys are working for me in zsh. Vim or emacs mode?

@Haprog
Copy link

Haprog commented May 19, 2020

@cantino I'm running this latest zsh branch (as I mentioned above linking the exact commit hash). I installed it with the instructions in the readme section Install manually from source

After having first installed the older version of this branch I just pulled latest updates (to get my branch up to date with latest changes) and ran cargo install --path . since I already have the relevant stuff in my PATH and in ~/.zshrc. Is there something else I should do?

The arrow keys still don't seem to work (I've also rebooted my laptop since last try).

By default Mcfly uses an emacs inspired key scheme. If you would like to switch to the vim inspired key scheme, set the environment variable MCFLY_KEY_SCHEME.

This is a bit confusing to me since I have no idea what is the difference and so I have just used the defaults without setting MCFLY_KEY_SCHEME manually. It would be great if there was at least some example of what keys are different when using McFly. I'm familiar with vim basics and do use vim a little bit but I've never used emacs.

I tried setting it to vim mode now but didn't notice a difference (except it says "McFly (Vim)" at the top when used), arrow keys still don't work.

@Haprog
Copy link

Haprog commented May 19, 2020

Can I switch between different versions just by switching the binary at ~/.cargo/bin/mcfly or does it depend on some other files?

I'm just thinking I could try compiling different versions from different commits and backup the binary after compilation to be able to switch back to better working version faster without having to run cargo install every time.

@Haprog
Copy link

Haprog commented May 19, 2020

I just noticed that if I manually run mcfly search then arrow keys work as they should, but if I start it with Ctrl+R the arrow keys don't work.

@Haprog
Copy link

Haprog commented May 19, 2020

I tested different versions of the mcfly.zsh file by checking out specific commits and starting a fresh terminal.

Arrow keys still work after Ctrl+R when using this commit:

But after the next commit the arrow keys don't work with Ctrl+R anymore:

@Haprog
Copy link

Haprog commented May 20, 2020

I just tested this branch also on Windows 10 running it with zsh in WSL1 (Ubuntu 20.04). Tested with two terminal apps: Command Prompt (cmd.exe, default) and the newer separately installed Windows Terminal. Both have the same issue for me that arrow keys don't work in McFly when starting it with Ctrl+R but works when started manually with command mcfly search.

And if I just downgrade the mcfly.zsh file to the version mentioned above, then arrow keys work as they should also on this Win 10 (but the issue with #mcfly: mcfly search appearing in terminal after usage is back though that is a lesser issue for me so I prefer that over having broken arrow keys).

@cantino
Copy link
Owner Author

cantino commented May 20, 2020

Thanks @Haprog for all of your testing! I'll see if I can reproduce this and figure it out.

@jrolfs
Copy link

jrolfs commented May 20, 2020

Tried mcfly @ 8eecd8d with...

name version
System macOS 10.15.4
Shell zsh 5.7.1 (x86_64-apple-darwin17.7.0)
Terminal kitty 0.17.1
Terminal Terminal.app 2.10 (433)

...and arrow keys don't work, but ctrl+n and ctrl+p do. Is that intentional? I personally prefer those bindings over the arrow keys so hopefully fixing the arrow keys doesn't make those go away ;)

Ah, I just tried invoking mcfly search . manually like @Haprog mentioned and I'm seeing the same behavior (the arrow keys work).

I also tried export MCFLY_KEY_SCHEME=vim and j/k work for up and down. This is a little confusing though as I do use ctrl+p/n all the time in vim.


Side note: in case anyone uses zplug, this is how I added mcfly:

# As a "plugin" to source key binding
zplug "cantino/mcfly", at:zsh, use:mcfly.zsh
# As a "command" to compile binary and add it to $PATH
zplug "cantino/mcfly", \
  at:zsh, \
  as:command, \
  use:"target/release/mcfly", \
  hook-build:"cargo install --path ."

@cantino
Copy link
Owner Author

cantino commented May 20, 2020

Thanks @jrolfs! This and the feedback from @Haprog is very helpful. I'll see if I can figure out how to fix it.

@Haprog
Copy link

Haprog commented May 21, 2020

@cantino I guess you could try to reproduce it in a fresh Ubuntu virtual machine if you can't reproduce it on your mac directly. You can get ready to go virtual machine images of Ubuntu for VirtualBox (free) or VMware from here https://www.osboxes.org/ubuntu/

@cantino
Copy link
Owner Author

cantino commented May 25, 2020

Okay, so this has something to do with normal mode vs application mode in the terminal. https://invisible-island.net/ncurses/ncurses.faq.html#cursor_appmode

I don't really understand it. If I do export TERM=vt220, arrows start working for mcfly in zsh.

What's strange is that they work find even without the TERM overloaded if you just run mcfly search directly, so zsh is either consuming the keys, or it's changing something about the terminal mode, when running the key binding. Any terminal experts here?

EDIT:

This definition of mcfly-history-widget seems to work!

  mcfly-history-widget() {
    () {
      local oldterm=$TERM
      TERM=vt220
      tput reset
      exec </dev/tty
      local mcfly_output=$(mktemp -t mcfly.output.XXXXXXXX)
      $MCFLY_PATH search -o "${mcfly_output}" "${LBUFFER}"
      local mode=$(sed -n 1p $mcfly_output)
      local selected=$(sed 1d $mcfly_output)
      rm -f $mcfly_output
      TERM=$oldterm
      if [[ -n $selected ]]; then
        RBUFFER=""
        LBUFFER="${selected}"
      fi
      if [[ "${mode}" == "run" ]]; then
        zle accept-line
      fi
      zle redisplay
    }
  }

EDIT 2: And, while this fixes it for my Ubuntu VM, it breaks it on my Mac. Any ideas?

EDIT 3: It appears to be just tput init that matters!

  mcfly-history-widget() {
    () {
      tput init
      exec </dev/tty
      local mcfly_output=$(mktemp -t mcfly.output.XXXXXXXX)
      $MCFLY_PATH search -o "${mcfly_output}" "${LBUFFER}"
      local mode=$(sed -n 1p $mcfly_output)
      local selected=$(sed 1d $mcfly_output)
      rm -f $mcfly_output
      if [[ -n $selected ]]; then
        RBUFFER=""
        LBUFFER="${selected}"
      fi
      if [[ "${mode}" == "run" ]]; then
        zle accept-line
      fi
      zle redisplay
    }
  }

I have noticed that mcfly is slower to come up in zsh and then bash, and I'm not sure why.

@cantino
Copy link
Owner Author

cantino commented May 25, 2020

@jrolfs & @Haprog, mind testing this again?

@infokiller
Copy link

@cantino I just took a look at all the comments since my last review.
Indeed, you shouldn't set TERM, as that will cause troubles. I would also not use tput init. Instead, you can try defining the following functions:

enable-term-application-mode() { echoti smkx }
disable-term-application-mode() { echoti rmkx }

Then, call disable-term-application-mode before you launch mcfly (which may fix the arrow keys issue), and enable-term-application-mode afterwards.

@Haprog
Copy link

Haprog commented May 27, 2020

@jrolfs & @Haprog, mind testing this again?

I just quickly tested the latest version and it seems to work perfectly. Thanks for the updates!

I have noticed that mcfly is slower to come up in zsh and then bash, and I'm not sure why.

At least I don't notice any delay when opening mcfly in zsh (though I'm on pretty high end hardware). It opens immediately for me. Maybe the slowness is only noticeable in a VM?

@cantino
Copy link
Owner Author

cantino commented May 28, 2020

@cantino I just took a look at all the comments since my last review.
Indeed, you shouldn't set TERM, as that will cause troubles. I would also not use tput init. Instead, you can try defining the following functions:

enable-term-application-mode() { echoti smkx }
disable-term-application-mode() { echoti rmkx }

Then, call disable-term-application-mode before you launch mcfly (which may fix the arrow keys issue), and enable-term-application-mode afterwards.

Awesome, that seems to be working! Mind testing again?

@cantino
Copy link
Owner Author

cantino commented May 28, 2020

Does the new version work for you @jrolfs?

@Haprog
Copy link

Haprog commented May 28, 2020

Still works great after the echoti update.

@cantino
Copy link
Owner Author

cantino commented Jun 3, 2020

I think this is mergeable now?

@orenyomtov
Copy link

Would love to see this merged

@cantino cantino merged commit 736db3d into master Jun 28, 2020
@cantino
Copy link
Owner Author

cantino commented Jun 28, 2020

Done! Releasing 0.4.0 now 🥳

@cantino cantino deleted the zsh branch June 28, 2020 18:26
@jrolfs
Copy link

jrolfs commented Jun 28, 2020

Sorry, I missed all these notifications somehow but glad y’all figured it out. Will switch over to master when I’m back on a 💻!

@Stormheg
Copy link

Stormheg commented Jul 2, 2020

Late to the party but just wanted to say thanks to everyone involved for adding zsh support!

Been using mcfly for a while now and love it ❤️ Thank you for creating such an amazing piece of software!

@cantino
Copy link
Owner Author

cantino commented Jul 4, 2020

Thanks @Stormheg ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not overwrite HISTFILE Zsh
7 participants