cantino / mcfly

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

Add compatibility with fish shell #95

Closed tjkirch closed 4 years ago

tjkirch commented 4 years ago

Implementation notes:

Possible future improvements:

Testing done:

Fixes cantino/mcfly#89.

tjkirch commented 4 years ago

^ This push rebases on master, per https://github.com/cantino/mcfly/pull/95#discussion_r461765585

tjkirch commented 4 years ago

^ This push addresses https://github.com/cantino/mcfly/pull/95#discussion_r461769704 by splitting HistoryFormat::Plain into Bash and Zsh, with extended_history only in Zsh. (This might also help to handle bash's HISTTIMEFORMAT in the future, similar to zsh's extended history.) It defaults to bash because that's probably most common, but the mcfly.*sh files all specify their format now.

scott2b commented 4 years ago

While this brings up McFly history in fish for me with ctrl^R, I am unable to get it to actually execute anything. Hitting either Tab or Return simply exits from McFly as if I had hit Esc.

tjkirch commented 4 years ago

While this brings up McFly history in fish for me with ctrl^R, I am unable to get it to actually execute anything. Hitting either Tab or Return simply exits from McFly as if I had hit Esc.

@scott2b What version of fish are you running?

It might help to comment out the removal of the temp file in mcfly.fish, then try again, and paste the content of that file here. Then we can see if the problem is inside mcfly or in mcfly.fish.

scott2b commented 4 years ago

I am using fish 3.1.2

My mcfly.output.... files consistently look like:

run
somecommand

Where somecommand is something that would have come from my history.

scott2b commented 4 years ago

Oh, but there's one file that is different. It is named mcfly.XXXXXXXX.g83UFnZd, note the missing .output. part of the name.

And the contents look more like a full history (it's about 10000 lines). E.g. a sample:

: 1592274472:0;mv 05* 05.Week05
: 1592274492:0;mv 06* 06.Week06
: 1592274504:0;mv 07* 07.Week07
: 1592274534:0;git add Lectures/02.Week02
: 1592274540:0;git add Lectures/03.Week03
: 1592274546:0;git add Lectures/04.Week04
: 1592274550:0;git add Lectures/05.Week05
tjkirch commented 4 years ago

My mcfly.output.... files consistently look like:

run
somecommand

Where somecommand is something that would have come from my history.

@scott2b That's the format of the McFly temp files before the change in this PR that makes them work with fish. Are you sure you built and installed McFly with the change from this PR, and put it in your PATH? The change isn't merged yet.

scott2b commented 4 years ago

I manually copied mcfly.fish into my fish functions directory and am sourcing that file and calling the binding function in my config.fish file.

Are you saying I should do a full install of mcfly from this branch?

tjkirch commented 4 years ago

I manually copied mcfly.fish into my fish functions directory and am sourcing that file and calling the binding function in my config.fish file.

Are you saying I should do a full install of mcfly from this branch?

Yeah, the Rust code changed too, so you have to build from this branch and use that McFly binary.

scott2b commented 4 years ago

Ok. My bad. I did not realize that. I should have dug deeper into the PR. Thank you for your time. I'll give that a shot.

scott2b commented 4 years ago

That was it! This is really great. Thank you so much @tjkirch

tjkirch commented 4 years ago

^ This push addresses @cantino's concerns by rebasing on master, then removing the MCFLY_PATH modifications in mcfly.bash (so there are no longer any changes in that file) and incorporates the path check from the bash/zsh files into mcfly.fish. Retested fish 2, fish 3, and bash OK.

cantino commented 4 years ago

Thanks @tjkirch! Sorry for the delay and thank you for your contribution!

zicklag commented 4 years ago

Yay! Thank you. Works in my fish shell. :smiley:

cantino commented 4 years ago

@tjkirch, I've been sometimes seeing commandline in my zsh output:

Screen Shot 2020-08-28 at 5 25 33 PM
tjkirch commented 4 years ago

@tjkirch, I've been sometimes seeing commandline in my zsh output:

Screen Shot 2020-08-28 at 5 25 33 PM

Interesting, I haven't seen that. I can't think of anything in the code that would insert "commandline" literally. I have a few thoughts:

cantino commented 4 years ago

So I think this was user error on my part. I haven't seen it again. Thanks @tjkirch!