Add official support for mksh

[saltymouse]

I'm getting the same error using mksh shell even with the new fnm release. Is there a possible similar fix for this shell?

thread 'main' panicked at 'Can't infer shell', src/shell/mod.rs:28:32
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Related: #296 #290

macOS 10.15.7 mksh: stable 59b fnm 1.22.3

[Schniz]

Hi @saltymouse and thanks for opening the issue!

Unfortunately, mksh is not officially supported right now.

You can add the --shell option explicitly to tell fnm to produce syntax that matches a different shell, like Bash:

eval "$(fnm env --shell=bash --use-on-cd)"

I verified it locally and it works. I'm open to a PR to make it officially supported, in which we can run all the e2e tests against it too.

From my local tests, looks like fnm env --shell=bash will work. I saw in the docs that mksh has hooks (which I don't understand how to use from the docs) and we can maybe find "the mksh way" to implement --use-on-cd, instead of overriding cd like we do in Bash's solution.

That being said, panicking is not a good way of saying that, so I'll work on a nicer error message.

Would you mind renaming this issue to Add official support for mksh?

[saltymouse]

Thanks for the info and the workaround. I'm not too familiar with the internals of mksh either, though I can ask the dev. Where should I look in the fnm code to reference making support for this new shell?

[Schniz]

• Here is the Bash support for fnm env: (which generates the commands in fnm env) https://github.com/Schniz/fnm/blob/master/src/shell/bash.rs
• Shell inference for Unix and for Windows (maybe we can have a single function that consolidates this logic that is duplicated in two files, something like impl From<&str> for Option<Box<dyn Shell>> implementation or to_supported_shell(binary_name: &str) -> Option<Box<dyn Shell>>)
• The tests/shellcode directory has lots of "high-level" shell DSL that should be implemented in order to make the e2e tests (that in tests/feature_tests) run against it.

IIUC from the docs, we can simply copy everything that Bash does and call it mksh, but I would really rather it to fit mksh like a glove 😄

[mirabilos]

Hi, mksh developer here.

I’ve been pinged in IRC and looked over https://github.com/Schniz/fnm/blob/master/src/shell/bash.rs and while I don’t understand all the things around the script, the script itself will work as well on mksh as it does on GNU bash. (mksh’s default rc file provides cd as function, so there’s no conflict either.)

In fact, all the extensions (relative to POSIX sh) used herein originated in the Korn Shell ☻

There are a number of problems with the script though:

• as izabera already pointed out, if cd fails, the remainder is still executed, and the errorlevel of the function is also 0
• if cd was an alias before the function definition (e.g. if this is run twice), bad things (recursion, death) will happen
• the checks themselves are a bit weird: [[ -f .nvmrc && .nvmrc ]] first checks whether a file called .nvmrc exists, then… checks whether the string .nvmrc has non-zero length (which is always true)

I’m proposing this to fix these issues; again, this goes for both GNU bash and mksh:

__fnmcd() {
	\cd "$@" || return $?
	if [[ -f .node-version || -f .nvmrc ]]; then
		fnm use
	fi
}
alias cd=__fnmcd

The backslash is no accident; it prevents expansion of “cd” as an alias during function definition time. (Aliases are never expanded during function runtime.)

---

As for the other things in that file; do note that both variables passed to set_env_var need quoting, as does the variable passed to path.

A quick but fully correct (though not minimal) way to quote an arbitrary string (including empty) for the shell goes thus:

1. replace all ' with '\''
2. prepend and append another ' each This also goes for POSIX sh.

---

I have no idea what you mean with shell interference, and even trying to look at the tests/shellcode directory gives me headaches.

But if you have specific shell snippets for me to look over, or concepts to explain, I’ll gladly help.

I did see one thing — there’s no such thing as shopt anywhere but in GNU bash.

[Schniz]

@mirabilos Thanks for the detailed answer!

I have no idea what you mean with shell interference, and even trying to look at the tests/shellcode directory gives me headaches.

Haha no worries. I mean shell inference, because fnm tries to understand which shell you're using by traversing the process tree. It isn't hard to add another shell to be inferred, but I want to know we support it before we allow fnm to automatically infer it.

To do so, I want to run all my e2e tests against the new shell, and it's mostly a non-issue — zsh and bash commands are almost identical but there are subtle differences, like there is no shopt like you mentioned in zsh, but I don't need aliases in Zsh because it has hooks.

I did see one thing — there’s no such thing as shopt anywhere but in GNU bash.

yup, and I see that in mksh aliases are expanded automatically even in non-interactive shells, which is great!

---

I think I can add mksh to the supported shells pretty easily.

thanks for the help again! 😄

[mirabilos]

Gal Schlezinger dixit:

@mirabilos Thanks for the detailed answer!

You’re welcome!

Haha no worries. I mean shell inference, because fnm tries to understand which shell you're using by traversing the process tree. It

Ah, okay. Hm, I don’t know if that is the best idea, as executable names aren’t necessarily fix (especially for the various NT versions… I have seen at least just “mksh” (mostly on Interix as the Explorer can’t start programs without .exe), “mksh.exe” and “mksh-w32.exe”).

Why not setup fnm for all shells, and then in the code that enables it pass the current shell type? Depending on the set of shells supported, detecting it can be trivial (GNU bash, mksh, zsh) or more complex; I’ve got a script that can detect really diverse shells as reference…

isn't hard to add another shell to be inferred, but I want to know we support it before we allow fnm to automatically infer it.

Indeed.

yup, and I see that in mksh aliases are expanded automatically even in non-interactive shells, which is great!

Yes, but remember they are expanded parse-time:

$ mksh <<\EOF
> alias meow='echo meow'
> foo() {
>         meow
> }
> typeset -f foo
> EOF
foo() {
	\echo meow
}

That is, the expanded definition has become part of the function definition, and changing the alias later will have no effect on the function foo.

mksh also ships a “dot.mkshrc” example file which many users will have loaded, which contains quite a number of aliases and functions; making sure an add-on works with and without it is sensible.

Good luck then, and if you have any other constructs you use to double-check, just write me; I don’t manage to extract everything from these .rs files (whatever language that is, never seen it before).