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

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

Merged
merged 3 commits into from
Jun 27, 2017
Merged

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

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0d2600
Auto-detect running editor on Windows for error overlay
levrik Jun 17, 2017
48bb51b
Ignore process output if powershell call fails
gaearon Jun 27, 2017
3ccd236
Support Notepad++
gaearon Jun 27, 2017
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
66 changes: 45 additions & 21 deletions packages/react-dev-utils/launchEditor.js
View file
Open in desktop
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,20 @@ 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'
];
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 +50,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,6 +60,7 @@ function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) {
case 'Atom Beta':
case 'subl':
case 'sublime':
case 'sublime_text':
case 'wstorm':
case 'appcode':
case 'charm':
Expand All @@ -68,6 +75,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 +100,37 @@ 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"').toString();
Copy link
Contributor

@gaearon gaearon Jun 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

What if powershell doesn't exist on the system? Seems like this prints stuff to console. Can we silence it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll push up a fix for this

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 +168,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 +200,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