Closed domoritz closed 3 years ago
Thanks for the report @domoritz. @b3nj5m1n, any ideas?
Does the new version work for anyone on fish shell?
I suppose the issue is that source
isn't is-interactive
. Also, it seems problematic to call mcfly_key_bindings
in non interactive scenarios.
I would probably change mcfly init fish
to not check for interactive and ask people to add
if status is-interactive
mcfly init fish | source
mcfly_key_bindings
end
or figure out a way to add to call mcfly_key_bindings
inside mcfly init fish
.
Sorry, I don't use fish shell. I think this is due to the changes made by @b3nj5m1n in #119, but I'm not sure. Does adding mcfly_key_bindings after like 77 of https://github.com/cantino/mcfly/blob/master/mcfly.fish help?
@tjkirch, any help on this would also be appreciated!
I'd personally stick with the approach from before https://github.com/cantino/mcfly/commit/23ea43b7c8fbd03a82ebd3d6fb74571d5e88f384. It's closer to standard and doesn't require any extra checks in the user's config.
I don't understand why there would be any difference between source "/usr/local/opt/mcfly/mcfly.fish"
and mcfly init fish | source
.
I think the problem is https://github.com/cantino/mcfly/blob/master/mcfly.fish#L4.
If I create a file test.fish
with test -t 0; or echo "no"
and then cat test.fish | source
I get no
.
Not sure what this really ensures and why we don't use https://fishshell.com/docs/current/cmds/isatty.html.
If I remove the line, it works. I also think it makes sense to add mcfly_key_bindings
to mcfly init fish
as recommended in https://github.com/cantino/mcfly/issues/129#issuecomment-796331393 since otherwise we would call mcfly_key_bindings
even in a non-interactive shell.
Would you be open to making a pull request that switches to isatty
and pulls mcfly_key_bindings
inside the file? I don't use fish so have somewhat low confidence that I'll get it right.
isatty
doesn't work either unfortunately.
$ cat test.fish
isatty stdin; or echo "no"
$ cat test.fish | source
no
$ source test.fish
$ isatty stdin; or echo "no"
I don't know enough about source and isatty is say whether this is a bug in fish (like https://github.com/fish-shell/fish-shell/issues/2477) or whether this is expected behavior.
@b3nj5m1n do you have a suggestion for how to fix McFly on fish? Otherwise, I will suggest to remove the isatty
check.
No, sorry but I don't use fish. I'd say try your approach.
I sent a pull request in https://github.com/cantino/mcfly/pull/137. It works for me.
I installed McFly via home-brew. I have version McFly 0.5.4. I also added
to the end of my
config.fish
file. I have fish version 3.2.0.Even when I run
mcfly init fish | source
in my terminal,mcfly_key_bindings
is still not defined.