gokcehan / lf

Terminal file manager
MIT License
7.64k stars 325 forks source link

Don't use SHELL env var by default #58

Closed occivink closed 6 years ago

occivink commented 7 years ago

Hi,

I think it might be better to just use /bin/sh (or whatever portable alternative) by default instead of $SHELL. Indeed, not all shells are posix-compliant (in particular fish, which I use) and quite a few things are broken as a result, which may be surprising to new users.

It should of course still be possible to override the shell with a setting in lfrc.

KenjiTakahashi commented 7 years ago

Perhaps /usr/bin/env sh? That's the de facto shebang for shell scripts that are wishing for portability. Would also handle sh (or whatever) not being in /bin (e.g. NixOS).

gokcehan commented 7 years ago

@occivink I guess we can do that. To be honest, I haven't been testing it with different shells anyways. Now that I think about it, two things come to mind that would not work with fish:

Is there anything else besides these two? I'm asking because without fixing these issues, there would be no way to use these shells with lf regardless of we change the default value or not.

Related, these things are very much broken in native windows as well (both cmd.exe and powershell).

@KenjiTakahashi How do they even survive without having /bin/sh? Almost all shell scripts I see has either /bin/sh or /bin/bash as the shebang line. Anyway, so I guess the question is, is it better to rely on having env in /usr/bin/env or sh in /bin/sh? Is there any distro which doesn't have /usr/bin/env installed?

occivink commented 7 years ago

Yes these are the two issues that I remember having.

I'm going to assume that the first is just a self-contained termbox issue that could be fixed. However the second is trickier, I see the following possibilities:

I think the second point would be okay. Even as a fish user, I can deal with making one-off sh scripts to be used in the context of lf. If I want to run a non-trivial command interactively I'll just drop into a real shell.

gokcehan commented 7 years ago

@occivink I agree second point looks like the best. Meanwhile, I have been trying to circumvent this limitation with something like:

cmd fish-example !{{
export args="$*"
fish -c '
    echo "this is running in fish"

    for i in (seq 5)
        echo $i
    end

    echo "fx =" $fx
    echo "argv =" $argv
    echo "args =" $args
'
}}

This works more or less, but one needs to be careful with quotes. I also tried to use heredoc literals like so:

cmd fish-example !{{
fish << EOF
    ..
EOF
}}

This would be a much cleaner solution but it doesn't work well for some reason.

@KenjiTakahashi Setting shell to /usr/bin/env sh needs some extra work so I used /bin/sh for now. Also, wiki page for shebang line mentions some systems have /bin/env instead of /usr/bin/env so maybe this is for the best. I suspect most people on these systems are used to doing such changes anyways.

occivink commented 7 years ago

@gokcehan I think you're entering escape-hell when nesting fish under sh. For example you can't simply iterate over the files in this fish instance with 'for f in $fs`. With how careful you have to be, it defeats the purpose of calling another shell imo.

gokcehan commented 7 years ago

@occivink This is not something I would use myself, just curious what things are possible. I remember kakoune have a similar design decision. Also I think heredoc literals are how python and others work in vim. I still need to see why it doesn't work well in lf currently.

KenjiTakahashi commented 7 years ago

@gokcehan I'm fine with this myself, just thought I'd mention there are systems which might have different dir layouts. In NixOS, /usr/bin/env is the only backwards compatibility they allowed (by default, it's literally the only file available in "standard" POSIX bin dirs). If packaged apps use different shebang, they are patched accordingly, etc. They chose the hard way of doing things :-). But I guess we can wait for someone actually using it daily to show up and then think about fixing it, if necessary.

gokcehan commented 7 years ago

@KenjiTakahashi I was wondering, can we directly use env or sh without the full path? After all, we are not using it as a shebang line so full path might not be needed at all. This way, env sh and sh should correspond to the same thing and so sh might be the reasonable choice as default.

KenjiTakahashi commented 7 years ago

Well, absolute path in shebangs is needed, because it is being run through execv calls, which do not respect environment setup (e.g. $PATH). Theoretically, shebang can use relative path, too, but it will only be searched relative to the pwd. But since we're actually calling it through Go's os/exec, relative paths should work as expected.

gokcehan commented 6 years ago

Changed the default shell from /bin/sh to only sh which seems to work without a problem. Closing this issue now. Feel free to report any problems.