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

Auto-detect running editor on Windows for error overlay #2552

Merged
merged 3 commits into from
Jun 27, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 52 additions & 21 deletions packages/react-dev-utils/launchEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
*/
'use strict';

var fs = require('fs');
var path = require('path');
var child_process = require('child_process');
var os = require('os');
var chalk = require('chalk');
var shellQuote = require('shell-quote');
const fs = require('fs');
const path = require('path');
const child_process = require('child_process');
const os = require('os');
const chalk = require('chalk');
const shellQuote = require('shell-quote');

function isTerminalEditor(editor) {
switch (editor) {
Expand All @@ -28,14 +28,21 @@ function isTerminalEditor(editor) {
// Map from full process name to binary that starts the process
// We can't just re-use full process name, because it will spawn a new instance
// of the app every time
var COMMON_EDITORS = {
const COMMON_EDITORS_OSX = {
'/Applications/Atom.app/Contents/MacOS/Atom': 'atom',
'/Applications/Atom Beta.app/Contents/MacOS/Atom Beta': '/Applications/Atom Beta.app/Contents/MacOS/Atom Beta',
'/Applications/Sublime Text.app/Contents/MacOS/Sublime Text': '/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl',
'/Applications/Sublime Text 2.app/Contents/MacOS/Sublime Text 2': '/Applications/Sublime Text 2.app/Contents/SharedSupport/bin/subl',
'/Applications/Visual Studio Code.app/Contents/MacOS/Electron': 'code',
};

const COMMON_EDITORS_WIN = [
'Code.exe',
'atom.exe',
'sublime_text.exe',
'notepad++.exe',
];
Copy link
Member

Choose a reason for hiding this comment

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

What about Visual Studio (devenv.exe)? It's the IDE for Windows. 😛

Copy link
Contributor

Choose a reason for hiding this comment

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

We're taking PRs 😛
VS was my first IDE...


function addWorkspaceToArgumentsIfExists(args, workspace) {
if (workspace) {
args.unshift(workspace);
Expand All @@ -44,7 +51,7 @@ function addWorkspaceToArgumentsIfExists(args, workspace) {
}

function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) {
var editorBasename = path.basename(editor).replace(/\.(exe|cmd|bat)$/i, '');
const editorBasename = path.basename(editor).replace(/\.(exe|cmd|bat)$/i, '');
switch (editorBasename) {
case 'vim':
case 'mvim':
Expand All @@ -54,11 +61,14 @@ function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) {
case 'Atom Beta':
case 'subl':
case 'sublime':
case 'sublime_text':
case 'wstorm':
case 'appcode':
case 'charm':
case 'idea':
return [fileName + ':' + lineNumber];
case 'notepad++':
return ['-n' + lineNumber, fileName];
case 'joe':
case 'emacs':
case 'emacsclient':
Expand All @@ -68,6 +78,7 @@ function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) {
case 'mine':
return ['--line', lineNumber, fileName];
case 'code':
case 'Code':
return addWorkspaceToArgumentsIfExists(
['-g', fileName + ':' + lineNumber],
workspace
Expand All @@ -92,21 +103,41 @@ function guessEditor() {
return shellQuote.parse(process.env.REACT_EDITOR);
}

// Using `ps x` on OSX we can find out which editor is currently running.
// Potentially we could use similar technique for Windows and Linux
if (process.platform === 'darwin') {
try {
var output = child_process.execSync('ps x').toString();
var processNames = Object.keys(COMMON_EDITORS);
for (var i = 0; i < processNames.length; i++) {
var processName = processNames[i];
// Using `ps x` on OSX or `Get-Process` on Windows we can find out which editor is currently running.
// Potentially we could use similar technique for Linux
try {
if (process.platform === 'darwin') {
const output = child_process.execSync('ps x').toString();
const processNames = Object.keys(COMMON_EDITORS_OSX);
for (let i = 0; i < processNames.length; i++) {
const processName = processNames[i];
if (output.indexOf(processName) !== -1) {
return [COMMON_EDITORS[processName]];
return [COMMON_EDITORS_OSX[processName]];
}
}
} else if (process.platform === 'win32') {
const output = child_process
.execSync('powershell -Command "Get-Process | Select-Object Path"', {
Copy link
Member

Choose a reason for hiding this comment

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

It's really annoying that Node.js doesn't have built-in abstractions for this. Hopefully better FFI support should make this more reliable at least, instead of having to parse strings.

stdio: ['pipe', 'pipe', 'ignore'],
})
.toString();
const runningProcesses = output.split('\r\n');
for (let i = 0; i < runningProcesses.length; i++) {
// `Get-Process` sometimes returns empty lines
if (!runningProcesses[i]) {
continue;
}

const fullProcessPath = runningProcesses[i].trim();
const shortProcessName = path.basename(fullProcessPath);

if (COMMON_EDITORS_WIN.indexOf(shortProcessName) !== -1) {
return [fullProcessPath];
}
}
} catch (error) {
// Ignore...
}
} catch (error) {
// Ignore...
}

// Last resort, use old skool env vars
Expand Down Expand Up @@ -144,7 +175,7 @@ function printInstructions(fileName, errorMessage) {
console.log();
}

var _childProcess = null;
let _childProcess = null;
function launchEditor(fileName, lineNumber) {
if (!fs.existsSync(fileName)) {
return;
Expand Down Expand Up @@ -176,7 +207,7 @@ function launchEditor(fileName, lineNumber) {
fileName = path.relative('', fileName);
}

var workspace = null;
let workspace = null;
if (lineNumber) {
args = args.concat(
getArgumentsForLineNumber(editor, fileName, lineNumber, workspace)
Expand Down