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.

Sign up for GitHub

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

Jump to bottom

[Feat] Add CUSTOM_EDITOR escape hatch for advanced configuration #39

Closed
wants to merge 5 commits into from
Closed

[Feat] Add CUSTOM_EDITOR escape hatch for advanced configuration #39

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c1a8c8
Add CUSTOM_EDITOR escape hatch for advanced configuration
Theo-Steiner May 9, 2022
1c13cf3
Merge branch 'master' into master
Theo-Steiner Jun 9, 2022
2b92888
Change CUSTOM_EDITOR var to LAUNCH_EDITOR
Theo-Steiner Aug 12, 2022
a860239
Merge upstream
Theo-Steiner Aug 12, 2022
528b592
Merge branch 'master' into master
Theo-Steiner Aug 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,9 @@ To launch files, send requests to the server like the following:
| `vim` | [Vim](http://www.vim.org/) |✓| | |
| `visualstudio` | [Visual Studio](https://www.visualstudio.com/vs/) | | |✓|
| `webstorm` | [WebStorm](https://www.jetbrains.com/webstorm/) |✓|✓|✓|

### Customization escape hatch

If your favorite combination of editor and os is not supported or you would like more advanced configuration options, a special environment variable allows for customization:

Simply set `LAUNCH_EDITOR` to the name of a script that is in your path, and the script will be called with `fileName`, `lineNumber` and `columnNumber` passed in as arguments.
3 changes: 3 additions & 0 deletions packages/launch-editor/get-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ module.exports = function getArgumentsForPosition (
case 'webstorm':
case 'webstorm64':
return ['--line', lineNumber, fileName]
// Just pass args if the user chose the LAUNCH_EDITOR escape hatch
case 'LAUNCH_EDITOR':
return [fileName, lineNumber, columnNumber]
}

// For all others, drop the lineNumber until we have
Expand Down
5 changes: 5 additions & 0 deletions packages/launch-editor/guess.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ module.exports = function guessEditor (specifiedEditor) {
// Ignore...
}

// Check if user chose the LAUNCH_EDITOR escape hatch
if (process.env.LAUNCH_EDITOR) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...?

Copy link
Contributor

@dominikg dominikg Aug 13, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return ['LAUNCH_EDITOR']
}

// Last resort, use old skool env vars
if (process.env.VISUAL) {
return [process.env.VISUAL]
Expand Down
9 changes: 8 additions & 1 deletion packages/launch-editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ function launchEditor (file, specifiedEditor, onErrorCallback) {

onErrorCallback = wrapErrorCallback(onErrorCallback)

const [editor, ...args] = guessEditor(specifiedEditor)
const result = guessEditor(specifiedEditor)
let [editor] = result
const [, ...args] = result

if (!editor) {
onErrorCallback(fileName, null)
return
Expand All @@ -104,6 +107,10 @@ function launchEditor (file, specifiedEditor, onErrorCallback) {
args.push(fileName)
}

if (editor === 'LAUNCH_EDITOR') {
editor = process.env.LAUNCH_EDITOR
}

if (_childProcess && isTerminalEditor(editor)) {
// There's an existing editor process already and it's attached
// to the terminal, so go kill it. Otherwise two separate editor
Expand Down