Fix various issues with packager editor launcher

Summary: There are a few small bugs with the code that launches the editor from the packager:

* First of all, the filepath is not escaped which means tokens like `(` or spaces will mess up the process execution. Dropbox unfortunately decided to use spaces in its enterprise product, so I was getting this error:

![screen shot 2015-07-11 at 3 20 54 pm](https://cloud.githubusercontent.com/assets/1135007/8635748/186e7f2e-27ea-11e5-8058-1f4dabb79634.png)

* Next, the line number argument formatting was assumed to be in a specific format (`:%d`) which actually errors out vim and other editors.
* Lastly, the process was started synchronously but not attached to the stdin / stdout of the parent process. This means that only editors like mvim, sublime, and others would work since they spawn a new window. Editors like emacs, vi, nano, etc wouldn't work and instead just hang at the command line.

So I whipped up this diff to fix a number of these issues, demo here:
http://recordit.co/M6zwiUj7hp

The demo shows both
Closes https://github.com/facebook/react-native/pull/1957

Reviewed By: @vjeux, @pcottle

Differential Revision: D2420941

Pulled By: @frantic
This commit is contained in:
Peter Cottle 2015-09-14 09:57:43 -07:00 committed by facebook-github-bot-7
parent 50dbbe5af2
commit c4c36116cf
1 changed files with 62 additions and 15 deletions

View File

@ -10,7 +10,37 @@
var chalk = require('chalk'); var chalk = require('chalk');
var fs = require('fs'); var fs = require('fs');
var exec = require('child_process').exec; var spawn = require('child_process').spawn;
function isTerminalEditor(editor) {
switch (editor) {
case 'vim':
case 'emacs':
case 'nano':
return true;
}
return false;
}
function getArgumentsForLineNumber(editor, fileName, lineNumber) {
switch (editor) {
case 'vim':
case 'mvim':
return [fileName, '+' + lineNumber];
case 'atom':
case 'subl':
case 'sublime':
return [fileName + ':' + lineNumber];
case 'joe':
case 'emacs':
return ['+' + lineNumber, fileName];
}
// For all others, drop the lineNumber until we have
// a mapping above, since providing the lineNumber incorrectly
// can result in errors or confusing behavior.
return [fileName];
}
function printInstructions(title) { function printInstructions(title) {
console.log([ console.log([
@ -25,28 +55,45 @@ function printInstructions(title) {
].join('\n')); ].join('\n'));
} }
var _childProcess = null;
function launchEditor(fileName, lineNumber) { function launchEditor(fileName, lineNumber) {
if (!fs.existsSync(fileName)) { if (!fs.existsSync(fileName)) {
return; return;
} }
var argument = fileName; var editor = process.env.REACT_EDITOR || process.env.EDITOR;
if (lineNumber) { if (!editor) {
argument += ':' + lineNumber; printInstructions('PRO TIP');
return;
} }
var editor = process.env.REACT_EDITOR || process.env.EDITOR; var args = [fileName];
if (editor) { if (lineNumber) {
args = getArgumentsForLineNumber(editor, fileName, lineNumber);
}
console.log('Opening ' + chalk.underline(fileName) + ' with ' + chalk.bold(editor)); console.log('Opening ' + chalk.underline(fileName) + ' with ' + chalk.bold(editor));
exec(editor + ' ' + argument, function(error) {
if (error) { if (_childProcess && isTerminalEditor(editor)) {
console.log(chalk.red(error.message)); // There's an existing editor process already and it's attached
printInstructions('How to fix'); // to the terminal, so go kill it. Otherwise two separate editor
// instances attach to the stdin/stdout which gets confusing.
_childProcess.kill('SIGKILL');
}
_childProcess = spawn(editor, args, {stdio: 'inherit'});
_childProcess.on('exit', function(errorCode) {
_childProcess = null;
if (errorCode) {
console.log(chalk.red('Your editor exited with an error!'));
printInstructions('Keep these instructions in mind:');
} }
}); });
} else {
printInstructions('PRO TIP'); _childProcess.on('error', function(error) {
} console.log(chalk.red(error.message));
printInstructions('How to fix:');
})
} }
module.exports = launchEditor; module.exports = launchEditor;