Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'shell' option #2547

Closed
wants to merge 1 commit into from

Conversation

@eraserhd
Copy link
Contributor

commented Nov 1, 2018

Rationale: On OS X, I don't have a decent way to replace the shell (I get
"Operation not permitted", even as root though I'd rather not much with
the system shell anyhow). But I'd like to use dash to speed things up,
and the current logic doesn't allow me to put it in the path or
anything.

src/shell_manager.hh Outdated Show resolved Hide resolved

@eraserhd eraserhd force-pushed the eraserhd:shell-option branch from 35437cd to 6d6ac17 Nov 2, 2018

@eraserhd

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

Also, documented option.

@mawww

This comment has been minimized.

Copy link
Owner

commented Nov 8, 2018

I am afraid by that option, sure we can document it to say it must be a posix shell, but I am afraid its going to be abused to run with fish, oilshell or something else, and that will lead to subtle (or not so subtle) breakage of scripts.

While I sympathize with the use case, I wonder if we cannot find another way to make overriding the shell possible without making it easier. (Keeping it "hard" is a good way to ensure people understand what they are doing).

@alexherbo2

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

How about using sh in PATH and /bin/sh as the falling case?

@Screwtapello

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

The issue is that the sh in $PATH may not be a POSIX shell. In particular, Solaris has a pre-POSIX /bin/sh. The reason Kakoune does the confstr(_CS_PATH) dance is to get a path that's guaranteed to have POSIX-compatible stuff in it.

How about, if ~/.config/kak/custom_shell exists at startup, that script is used as the shell for that session. It's still pretty easy to do, but if it's not a standard option it will be harder to find. On the other hand, if somebody forgets they've configured Kakoune to use a custom shell and they uninstall that shell, breaking Kakoune, they're probably going to look in ~/.config/kak and see the custom_shell file there. It might even be a broken symlink, which usually gets highlighted in a scary colour, if you're using GNU coreutils.

Bonus points: if the custom_shell exists but is not executable, put a warning in *debug*.

@maximbaz

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

I don't think you should introduce weird alternative ways to configure kakoune, name the option posix_shell instead of shell to make it very clear that it needs to be POSIX-compliant, and let people handle the rest. It's not your fault if someone wants to mess with options, and besides, nobody will be replacing shell with fish anyway, people just want to use dash to workaround the absence of lazy loading.

@eraserhd

This comment has been minimized.

Copy link
Contributor Author

commented Nov 8, 2018

This doesn't really help lazy loading, since the user's kakrc (which is presumably where you'd have to set your shell option) loads after all the autoload scripts.

I'm trying to make my parinfer plugin work faster, though I can't even test this because dash on Mac OS X is super broken. Let me try installing an older version today.

@maximbaz

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

Hopefully some kind of this alias will help you to speed up kakoune startup while we wait for lazy loading:
alias kak='kak -e "set global posix_shell /bin/dash"'

@maximbaz

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

But on the other hand, having an option that changes shell in the middle of execution (so that some scripts are executed with one shell and then others are executed with another shell) sounds a bit dangerous. Because of this, maybe kakoune instead should support some KAKOUNE_POSIX_SHELL environment variable that would contain path to the shell to use.

@lenormf

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

Make the shell configurable through a pre-processing option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.