[Feat] Add CUSTOM_EDITOR escape hatch for advanced configuration

[Theo-Steiner]

TL;DR: Provide an escape hatch for more customization, via a CUSTOM_EDITOR environment variable that points to a script, which is passed fileName, lineNumber and columnNumber.

First of all: thank you for creating this package, I started using it as an upstream dependency of vite-plugin-svelte-inspector today and am amazed by how cool this is!
Since neovim is my editor of choice, I started tinkering away and ended up with an implementation that works okay-ish for me, but is very hacky.

Just by setting VISUAL="nvim" it already sort of works, and node spawns a subprocess with nvim opening the correct file, but everything breaks on subsequent clicks, because having multiple instances of nvim run as subprocesses is obviously not optimal and even when using VISUAL="vim", which should be better as launch-editor detects that it's a terminal editor and kills the process once before spawning it again, after a few edits everything breaks in ugly ways.
So I put a script into my path that uses neovim-remote (https://github.com/mhinz/neovim-remote) to remotely open the file in a specific neovim instance that was listening for remote commands (nvim --listen /tmp/nvimsocket) like this
nvr --remote "$@" and again it sort of worked, opening the correct file provided by Inspector-click in the "server" neovim instance, but not supporting line/column number. (As this being passed on depends on a hardcoded list of editors in get-args.js and the script name I picked obviously wasn't in this hardcoded list.)
Since I don't use mvim (one of the arbitrarily chosen supported editors) I just decided to call my command to open the neovim remote mvim, and this resulted in line number and character positions being accurately passed to my script.

Now, this is hacky and not nice on two levels: 1. VISUAL/EDITOR have to be set to a command that is not necessarily the same command one would use for git operations for example 2. I had to resort to naming my command mvim to receive the correct arguments, which is only possible because I don't use mvim and I happened to need the arguments in the same manner.
Since it is impossible to support every conceivable editors in this package, it might be nice to provide an escape hatch for people to write custom scripts to open the editor they want, in the way they want.
This PR proposes introducing an escape hatch for more customization in form of a CUSTOM_EDITOR environment variable that can specify a command to be called with fileName, lineNumber and columnNumber passed in as arguments, which can then be used however the user wishes.

[dominikg]

@sodatea could you check this out if you find the time? It would provide workarounds/solutions to various issues related to opening an editor and enhance the user experience in vite.

possibly affects #38 #35 #16 #15 #11 #9 #7

[chrisfromredfin]

I have an odd but specific use case in mind that I think this would definitely be the solution for, so 👍🏼

[Suyashtnt]

Could you share your mvim script? I would like to be able to send stuff to my remote neovim process while I wait for this PR to get merged

[Theo-Steiner]

Could you share your mvim script? I would like to be able to send stuff to my remote neovim process while I wait for this PR to get merged

sure! it's as simple as

# ~/.zsh/mvim
nvr --remote "$@"

and

# ~/.zshrc
export VISUAL="mvim"
# then I use this to open up my project. 
# "h(ost)vim" => wrapper around nvim that listens to neovim-remote's standard server location
hvim() {
    nvim --listen /tmp/nvimsocket "$@"
}

[hixb]

or not

[sodatea]

Thanks! I finally got the time to read through the issues and PRs. And I've discussed this one with Evan.
We agree that it's worth having an escape hatch for custom editors.

One nitpick: considering this environment variable is only used for this package, the name CUSTOM_EDITOR seems too general. How about changing it to something more specific, like LAUNCH_EDITOR?

[chrisfromredfin]

👍🏼 +1 to LAUNCH_EDITOR which seems more specific.

[Theo-Steiner]

I agree, LAUNCH_EDITOR is a better name!

[dominikg]

should this be done at the beginning of "guess" or even outside of it, it isn't a guess if the user specified an explicit command to use.

In fact if a different editor was found in the process list it would still be preferred over LAUNCH_EDITOR in the current implementation

(sorry for not asking this sooner)

[Theo-Steiner]

I made it like this to have the changes be as low profile as possible. This way it would definitely not disturb how launch-editor operated before and be non-breaking.
I guess the amount of people who use this tool already and happen to have a LAUNCH_EDITOR environment variable set is pretty negligible though...?

[dominikg]

Technically it would be a breaking change either way for people that already have the environment variable set. a google search for "LAUNCH_EDITOR" did turn up very few results (<3k) and skimming them i didn't find documented use of it as an env variable

For extra safety, LAUNCH_EDITOR_COMMAND has 3 results and all of them seem to point at one python lib config option from 2018.

[dominikg]

Another question. Should get-args"be used to try and pass args to known editors before resorting to 3 different args?

That would allow LAUNCH_EDITOR=code to work without users having to create a wrapper script

[dominikg]

minimal changes would be like this: https://github.com/yyx990803/launch-editor/compare/master...dominikg:launch-editor:feat/launch-editor-env?expand=1

Q: Do we need to call shell-quote on the content of LAUNCH_EDITOR? (not done on the content of EDITOR or VISUAL, but was wondering since it is used for specifiedEditor, which is basically the js equivalent of LAUNCH_EDITOR)

Q: Would specifiedEditor or LAUNCH_EDITOR have precedence

[bodograumann]

Btw. here is another fork of react-dev-utils functionality as a vite plugin (and uses the VUE_EDITOR env variable):

• https://github.com/webfansplz/vite-plugin-vue-inspector
• https://github.com/webfansplz/vite-plugin-vue-inspector/blob/main/src/launch-editor.ts#L193-L200

[dominikg]

Btw. here is another fork of react-dev-utils functionality as a vite plugin (and uses the VUE_EDITOR env variable):

• https://github.com/webfansplz/vite-plugin-vue-inspector
• https://github.com/webfansplz/vite-plugin-vue-inspector/blob/main/src/launch-editor.ts#L193-L200

Do you want to suggest that vite (or others using this lib) should define their own env variables for custom support?
This could get confusing fast... VUE_EDITOR, REACT_EDITOR SVELTE_EDITOR ... and everyone has to bring their own middlewares to vite. Much easier to have one generic env like suggested here. Then the vue-inspector can do what the svelte-inspector already does and just call vite's own middleware https://github.com/sveltejs/vite-plugin-svelte/blob/main/packages/vite-plugin-svelte/src/ui/inspector/Inspector.svelte#L74

[dominikg]

see #49 for the changes i suggested above. Obviously i prefer the added flexibility it brings but would be fine with the implementation here too.

[bodograumann]

No, I don't care about that, @dominikg. Not sure where you get that from.
I only mentioned the existing implementation, because that is in which context I found this project
and it might serve as inspiration, because it already implements user override.

What I would be concerned about, is discoverability of this feature when used e.g. in vite.
Maybe it would be good to add some documentation to vite then, but first the feature needs to completed here.

[sodatea]

Thanks for the helpful discussions!
I've merged @dominikg 's #49 instead of this one as I agree that it's more minimal.

As for the unanswered questions:

Q: Do we need to call shell-quote on the content of LAUNCH_EDITOR? (not done on the content of EDITOR or VISUAL, but was wondering since it is used for specifiedEditor, which is basically the js equivalent of LAUNCH_EDITOR)

I think we don't need to bother until there are users complaining.

Q: Would specifiedEditor or LAUNCH_EDITOR have precedence

The current implementation looks good to me, i.e. specifiedEditor should take precedence, otherwise, the new release could surprise the downstream dependents.

[bodograumann]

Note: This feature (now implemented by #49) will be included in the upcoming vite 3.1.0 release, which is currently in beta.