cantino / mcfly

Fly through your shell history. Great Scott!
MIT License
6.9k stars 178 forks source link

Mac issues #6

Open f355 opened 5 years ago

f355 commented 5 years ago

Great idea! Tried it on my Mac with bash from homebrew (4.4.23(1)-release) and faced a few issues:

  1. tr fails here: https://github.com/cantino/mcfly/blob/master/mcfly.bash#L8 with tr: Illegal byte sequence. Seems like LC_CTYPE is not enough, need to set LC_ALL=C, it seems to work that way.

  2. After pressing ctrl-r and ctrl-g to get back to the shell, I see this:

    [2018-12-04 02:25:56][prod] konstantin:~
    $ #mcfly:
    [2018-12-04 02:25:59][prod] konstantin:~
    $  mcfly search
    [2018-12-04 02:26:06][prod] konstantin:~
    $

    Looks like in this line: https://github.com/cantino/mcfly/blob/master/mcfly.bash#L47 it is executing commands quite literally in the shell, displaying what it's doing. Not sure if it's a mac funkiness or my particular setup or what, but it's annoying.

  3. In standard bash, after pressing ctrl-r and typing something I can keep scrolling back in time by pressing ctrl-r again, and I have a spinal reflex for that. mcfly just drops me back out to the shell, rendering itself effectively unusable for me.

Thanks for your effort. I will try to keep using it, despite the third point, and report what I find.

cantino commented 5 years ago

Hey @f355, thanks for testing this out!

  1. That's odd. So does changing it to export MCFLY_SESSION_ID=$(cat /dev/urandom | env LC_ALL=C LC_CTYPE=C tr -dc 'a-zA-Z0-9' | fold -w 24 | head -n 1) fix it for you?
  2. The #mcfly: part is an unavoidable (as far as I can tell) part of how you have to hook into the Bash shell. I need to escape any special characters so that they don't execute, and as far as I can tell, the only way to do this with readline is to comment them out.
  3. That's a great point. I'd be open to changing the default so that further ctrl-r hits move you through the history in the UI.
f355 commented 5 years ago
  1. Yes. We can compare our set-ups if you want, but I don't have anything too special with regards to locale.
  2. The problem is that it prints those two lines out. Is it unavoidable? I'll try to research more and maybe PR if I find anything.
  3. Thank you :)

While we're at it, I've found out a couple other issues:

  1. Similar to 3 above, ctrl+f in standard bash/readline works just like Tab in mcfly, and turns out I also have a spinal reflex for it. Funny how you never think about these things until they change.
  2. The color scheme seems totally buggered for me, both in iTerm2 with Solarized Light theme from here: https://github.com/altercation/solarized/tree/master/iterm2-colors-solarized, as well as Terminal.app with basic colors. Here are some screenshots: https://imgur.com/a/GXiooGV
cantino commented 5 years ago

If LC_ALL=C fixes it, mind sending a PR?

Eww, those colors definitely don't look good on a light background! Guess we need to make them customizable.

f355 commented 5 years ago

Sure, my pleasure: #7

hkjels commented 5 years ago

Got the same issue, but LC_ALL=C didn't solve it. Could any other such preference affect tr?

pckilgore commented 5 years ago

+1 on issue number two. There's got to be a way to kill the line from bash and yank it back as an argument to mcfly (which appears to be what the script is doing). I'll dig through the bash docs if I have time. I can make another issue for it, your call @hkjels

cantino commented 5 years ago

If you can figure it out, that'd be great!

f355 commented 5 years ago

Looks like bind -x and READLINE_LINE/READLINE_POINT environment variables are intended for this exact purpose (from here):

-x keyseq:shell-command Cause shell-command to be executed whenever keyseq is entered. When shell-command is executed, the shell sets the READLINE_LINE variable to the contents of the Readline line buffer and the READLINE_POINT variable to the current location of the insertion point. If the executed command changes the value of READLINE_LINE or READLINE_POINT, those new values will be reflected in the editing state.

cantino commented 5 years ago

Good find @f355! This could be a much better solution.