Open editor to exact column from build error overlay (facebook#3465)

* Open editor to exact column from build error overlay * Update launch editor validations
Pavek · Jul 10, 2018 · c905165 · c905165
1 parent 4c9022e
commit c905165
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 10 deletions.
diff --git a/packages/react-dev-utils/errorOverlayMiddleware.js b/packages/react-dev-utils/errorOverlayMiddleware.js
@@ -12,7 +12,9 @@ const launchEditorEndpoint = require('./launchEditorEndpoint');
 module.exports = function createLaunchEditorMiddleware() {
   return function launchEditorMiddleware(req, res, next) {
     if (req.url.startsWith(launchEditorEndpoint)) {
-      launchEditor(req.query.fileName, req.query.lineNumber);
+      const lineNumber = parseInt(req.query.lineNumber, 10) || 1;
+      const colNumber = parseInt(req.query.colNumber, 10) || 1;
+      launchEditor(req.query.fileName, lineNumber, colNumber);
       res.end();
     } else {
       next();

diff --git a/packages/react-dev-utils/launchEditor.js b/packages/react-dev-utils/launchEditor.js
@@ -103,7 +103,13 @@ function addWorkspaceToArgumentsIfExists(args, workspace) {
   return args;
 }
 
-function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) {
+function getArgumentsForLineNumber(
+  editor,
+  fileName,
+  lineNumber,
+  colNumber,
+  workspace
+) {
   const editorBasename = path.basename(editor).replace(/\.(exe|cmd|bat)$/i, '');
   switch (editorBasename) {
     case 'atom':
@@ -112,17 +118,19 @@ function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) {
     case 'subl':
     case 'sublime':
     case 'sublime_text':
+      return [fileName + ':' + lineNumber + ':' + colNumber];
     case 'wstorm':
     case 'charm':
       return [fileName + ':' + lineNumber];
     case 'notepad++':
-      return ['-n' + lineNumber, fileName];
+      return ['-n' + lineNumber, '-c' + colNumber, fileName];
     case 'vim':
     case 'mvim':
     case 'joe':
+      return ['+' + lineNumber, fileName];
     case 'emacs':
     case 'emacsclient':
-      return ['+' + lineNumber, fileName];
+      return ['+' + lineNumber + ':' + colNumber, fileName];
     case 'rmate':
     case 'mate':
     case 'mine':
@@ -132,7 +140,7 @@ function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) {
     case 'code-insiders':
     case 'Code - Insiders':
       return addWorkspaceToArgumentsIfExists(
-        ['-g', fileName + ':' + lineNumber],
+        ['-g', fileName + ':' + lineNumber + ':' + colNumber],
         workspace
       );
     case 'appcode':
@@ -255,17 +263,24 @@ function printInstructions(fileName, errorMessage) {
 }
 
 let _childProcess = null;
-function launchEditor(fileName, lineNumber) {
+function launchEditor(fileName, lineNumber, colNumber) {
   if (!fs.existsSync(fileName)) {
     return;
   }
 
   // Sanitize lineNumber to prevent malicious use on win32
   // via: https://github.com/nodejs/node/blob/c3bb4b1aa5e907d489619fb43d233c3336bfc03d/lib/child_process.js#L333
-  if (lineNumber && isNaN(lineNumber)) {
+  // and it should be a positive integer
+  if (!(Number.isInteger(lineNumber) && lineNumber > 0)) {
     return;
   }
 
+  // colNumber is optional, but should be a positive integer too
+  // default is 1
+  if (!(Number.isInteger(colNumber) && colNumber > 0)) {
+    colNumber = 1;
+  }
+
   let [editor, ...args] = guessEditor();
 
   if (!editor) {
@@ -294,7 +309,13 @@ function launchEditor(fileName, lineNumber) {
   let workspace = null;
   if (lineNumber) {
     args = args.concat(
-      getArgumentsForLineNumber(editor, fileName, lineNumber, workspace)
+      getArgumentsForLineNumber(
+        editor,
+        fileName,
+        lineNumber,
+        colNumber,
+        workspace
+      )
     );
   } else {
     args.push(fileName);

diff --git a/packages/react-dev-utils/webpackHotDevClient.js b/packages/react-dev-utils/webpackHotDevClient.js
@@ -30,7 +30,9 @@ ErrorOverlay.setEditorHandler(function editorHandler(errorLocation) {
       '?fileName=' +
       window.encodeURIComponent(errorLocation.fileName) +
       '&lineNumber=' +
-      window.encodeURIComponent(errorLocation.lineNumber || 1)
+      window.encodeURIComponent(errorLocation.lineNumber || 1) +
+      '&colNumber=' +
+      window.encodeURIComponent(errorLocation.colNumber || 1)
   );
 });
 

diff --git a/packages/react-error-overlay/src/utils/parseCompileError.js b/packages/react-error-overlay/src/utils/parseCompileError.js
@@ -4,6 +4,7 @@ import Anser from 'anser';
 export type ErrorLocation = {|
   fileName: string,
   lineNumber: number,
+  colNumber?: number,
 |};
 
 const filePathRegex = /^\.(\/[^/\n ]+)+\.[^/\n ]+$/;
@@ -25,6 +26,7 @@ function parseCompileError(message: string): ?ErrorLocation {
   const lines: Array<string> = message.split('\n');
   let fileName: string = '';
   let lineNumber: number = 0;
+  let colNumber: number = 0;
 
   for (let i = 0; i < lines.length; i++) {
     const line: string = Anser.ansiToText(lines[i]).trim();
@@ -41,6 +43,8 @@ function parseCompileError(message: string): ?ErrorLocation {
       const match: ?Array<string> = line.match(lineNumberRegexes[k]);
       if (match) {
         lineNumber = parseInt(match[1], 10);
+        // colNumber starts with 0 and hence add 1
+        colNumber = parseInt(match[2], 10) + 1 || 1;
         break;
       }
       k++;
@@ -51,7 +55,7 @@ function parseCompileError(message: string): ?ErrorLocation {
     }
   }
 
-  return fileName && lineNumber ? { fileName, lineNumber } : null;
+  return fileName && lineNumber ? { fileName, lineNumber, colNumber } : null;
 }
 
 export default parseCompileError;