cantino / mcfly

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

Avoid TIOCSTI on Bash #416

Closed jtschuster closed 4 weeks ago

jtschuster commented 1 month ago

To avoid using TIOCSTI on bash, mcfly can utilize the readline bindings. However, bind only lets you bind either a bash command / function or a readline function, but we can bind Ctrl+R to two keybindings - one which runs mcfly and binds/unbinds the second key to the accept-line readline depending on if enter or tab is pressed. It's a bit of a hack, so there's an escape hatch if a user prefers to use TIOCSTI.

cantino commented 1 month ago

Fascinating approach, I'll have to play with this and get back to you, thank you @jtschuster!

cantino commented 1 month ago

It seems to basically work in bash on Mac and Linux. On Linux I am getting the following error during script setup: bash: [: =: unary operator expected

jtschuster commented 1 month ago

Thanks for the heads up, missed that in my testing. Looks like it was an issue with using single brackets in the if statements, should be fixed now.

nuke-web3 commented 1 month ago

In #404 running cargo r -r -- -d search git for this branch brings up the TUI to select history correctly, but with sudo sysctl -w dev.tty.legacy_tiocsti=0 set now tab does not fill the command in the tty. (same behavior as before, I have not debuged if it's also from the same panic)

The same code referencing TIOCSTI exists in this branch that was panicing, so I am not sure this is a complete fix?

https://github.com/cantino/mcfly/blob/caad8168c407ee239e61ec10e4e02101a77cd0e4/src/fake_typer.rs#L10-L19

cantino commented 1 month ago

@nuke-web3 this will only work if executed from bash using the key bindings, I think.

jamedeus commented 1 month ago

Works on Ubuntu 24.04 server and desktop (bash 5.2.21, kernel 6.8.0-31)

cantino commented 4 weeks ago

Works for me in linux and mac bash.

@jtschuster do you think there's any avenue for fixing it for manual executions of mcfly search? I suspect no.

jtschuster commented 4 weeks ago

No, I couldn't find any drop-in replacement for it.

cantino commented 4 weeks ago

This is a good solution, thank you @jtschuster!