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:
parent
515d5a5f4b
commit
c372dab213
|
@ -10,7 +10,37 @@
|
|||
|
||||
var chalk = require('chalk');
|
||||
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) {
|
||||
console.log([
|
||||
|
@ -25,28 +55,45 @@ function printInstructions(title) {
|
|||
].join('\n'));
|
||||
}
|
||||
|
||||
var _childProcess = null;
|
||||
function launchEditor(fileName, lineNumber) {
|
||||
if (!fs.existsSync(fileName)) {
|
||||
return;
|
||||
}
|
||||
|
||||
var argument = fileName;
|
||||
if (lineNumber) {
|
||||
argument += ':' + lineNumber;
|
||||
var editor = process.env.REACT_EDITOR || process.env.EDITOR;
|
||||
if (!editor) {
|
||||
printInstructions('PRO TIP');
|
||||
return;
|
||||
}
|
||||
|
||||
var editor = process.env.REACT_EDITOR || process.env.EDITOR;
|
||||
if (editor) {
|
||||
console.log('Opening ' + chalk.underline(fileName) + ' with ' + chalk.bold(editor));
|
||||
exec(editor + ' ' + argument, function(error) {
|
||||
if (error) {
|
||||
console.log(chalk.red(error.message));
|
||||
printInstructions('How to fix');
|
||||
}
|
||||
});
|
||||
} else {
|
||||
printInstructions('PRO TIP');
|
||||
var args = [fileName];
|
||||
if (lineNumber) {
|
||||
args = getArgumentsForLineNumber(editor, fileName, lineNumber);
|
||||
}
|
||||
console.log('Opening ' + chalk.underline(fileName) + ' with ' + chalk.bold(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
|
||||
// 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:');
|
||||
}
|
||||
});
|
||||
|
||||
_childProcess.on('error', function(error) {
|
||||
console.log(chalk.red(error.message));
|
||||
printInstructions('How to fix:');
|
||||
})
|
||||
}
|
||||
|
||||
module.exports = launchEditor;
|
||||
|
|
Loading…
Reference in New Issue