From 7bcf470f6fc2bc0e25b369de36ee63b58c475bde Mon Sep 17 00:00:00 2001 From: Tharaka Wijebandara Date: Sun, 19 Nov 2017 10:06:35 +0530 Subject: [PATCH 1/2] Open editor to exact column from build error overlay --- .../react-dev-utils/errorOverlayMiddleware.js | 6 +++- packages/react-dev-utils/launchEditor.js | 30 +++++++++++++++---- .../react-dev-utils/webpackHotDevClient.js | 4 ++- .../src/utils/parseCompileError.js | 6 +++- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/react-dev-utils/errorOverlayMiddleware.js b/packages/react-dev-utils/errorOverlayMiddleware.js index b756b0ef647..19fa73c24c5 100644 --- a/packages/react-dev-utils/errorOverlayMiddleware.js +++ b/packages/react-dev-utils/errorOverlayMiddleware.js @@ -12,7 +12,11 @@ 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); + launchEditor( + req.query.fileName, + req.query.lineNumber, + req.query.colNumber + ); res.end(); } else { next(); diff --git a/packages/react-dev-utils/launchEditor.js b/packages/react-dev-utils/launchEditor.js index ba16827e644..1d14b3b0f05 100644 --- a/packages/react-dev-utils/launchEditor.js +++ b/packages/react-dev-utils/launchEditor.js @@ -95,7 +95,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': @@ -104,17 +110,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': @@ -122,7 +130,7 @@ function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) { case 'code': case 'Code': return addWorkspaceToArgumentsIfExists( - ['-g', fileName + ':' + lineNumber], + ['-g', fileName + ':' + lineNumber + ':' + colNumber], workspace ); case 'appcode': @@ -245,7 +253,7 @@ function printInstructions(fileName, errorMessage) { } let _childProcess = null; -function launchEditor(fileName, lineNumber) { +function launchEditor(fileName, lineNumber, colNumber) { if (!fs.existsSync(fileName)) { return; } @@ -256,6 +264,10 @@ function launchEditor(fileName, lineNumber) { return; } + // colNumber is optional, but should be a number + // default is 1 + colNumber = parseInt(colNumber, 10) || 1; + let [editor, ...args] = guessEditor(); if (!editor) { printInstructions(fileName, null); @@ -279,7 +291,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 index 296e380467d..cbbc80029ae 100644 --- 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 index 2c9b6e60ebb..e87c45622c1 100644 --- 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 = 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 = 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; From 2e346d4d763d1bbbf9122398a2e1827a194f3a2e Mon Sep 17 00:00:00 2001 From: Tharaka Wijebandara Date: Sun, 17 Dec 2017 00:08:52 +0530 Subject: [PATCH 2/2] Update launch editor validations --- packages/react-dev-utils/errorOverlayMiddleware.js | 8 +++----- packages/react-dev-utils/launchEditor.js | 9 ++++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/react-dev-utils/errorOverlayMiddleware.js b/packages/react-dev-utils/errorOverlayMiddleware.js index 19fa73c24c5..873b1994732 100644 --- a/packages/react-dev-utils/errorOverlayMiddleware.js +++ b/packages/react-dev-utils/errorOverlayMiddleware.js @@ -12,11 +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, - req.query.colNumber - ); + 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 index 1d14b3b0f05..9e9e2b79149 100644 --- a/packages/react-dev-utils/launchEditor.js +++ b/packages/react-dev-utils/launchEditor.js @@ -260,13 +260,16 @@ function launchEditor(fileName, lineNumber, colNumber) { // 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 number + // colNumber is optional, but should be a positive integer too // default is 1 - colNumber = parseInt(colNumber, 10) || 1; + if (!(Number.isInteger(colNumber) && colNumber > 0)) { + colNumber = 1; + } let [editor, ...args] = guessEditor(); if (!editor) {