From ae20c769485993d6ff69fcf37e1027905a3076da Mon Sep 17 00:00:00 2001 From: Toby Ho Date: Thu, 1 Aug 2019 15:47:15 -0400 Subject: [PATCH 1/3] More input lines displayed on error for context and line numbers. --- moo.js | 27 +++++++++++++++++++++------ test/test.js | 21 +++++++++++++-------- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/moo.js b/moo.js index cf3a27e..77041cd 100644 --- a/moo.js +++ b/moo.js @@ -530,7 +530,9 @@ // throw, if no rule with {error: true} if (group.shouldThrow) { - throw new Error(this.formatError(token, "invalid syntax")) + var err = new Error(this.formatError(token, "invalid syntax")) + err.token = token + throw err; } if (group.pop) this.popState() @@ -571,13 +573,26 @@ col: this.col, } } - var start = Math.max(0, token.offset - token.col + 1) - var eol = token.lineBreaks ? token.text.indexOf('\n') : token.text.length - var firstLine = this.buffer.substring(start, token.offset + eol) + + var lines = this.buffer.split("\n") + .slice( + Math.max(0, token.line - 5), + token.line + ); + var lastLineDigits = String(token.line).length message += " at line " + token.line + " col " + token.col + ":\n\n" - message += " " + firstLine + "\n" - message += " " + Array(token.col).join(" ") + "^" + message += lines + .map(function(line, i) { + return pad(token.line - lines.length + i + 1, lastLineDigits) + " " + line; + }) + .join("\n") + message += "\n" + pad("", lastLineDigits + token.col) + "⬆︎\n" return message + + function pad(n, length) { + var s = String(n) + return Array(length - s.length + 1).join(" ") + s + } } Lexer.prototype.clone = function() { diff --git a/test/test.js b/test/test.js index 2e20d18..dd94af3 100644 --- a/test/test.js +++ b/test/test.js @@ -905,8 +905,9 @@ describe('errors', () => { expect(lexer.next()).toMatchObject({value: '456'}) expect(() => lexer.next()).toThrow( "invalid syntax at line 2 col 4:\n\n" + - " 456baa\n" + - " ^" + "1 123\n" + + "2 456baa\n" + + " ⬆︎" ) }) @@ -921,8 +922,10 @@ describe('errors', () => { expect(tok).toMatchObject({type: 'error', value: ' 12\n345\n6', lineBreaks: 2}) expect(lexer.formatError(tok, "numbers!")).toBe( "numbers! at line 3 col 2:\n\n" + - " g 12\n" + - " ^" + "1 abc\n" + + "2 def\n" + + "3 g 12\n" + + " ⬆︎\n" ) }) @@ -937,8 +940,9 @@ describe('errors', () => { expect(lexer.col).toBe(9) expect(lexer.formatError(undefined, "EOF!")).toBe( "EOF! at line 2 col 9:\n\n" + - " def quxx\n" + - " ^" + "1 abc\n" + + "2 def quxx\n" + + " ⬆︎\n" ) }) @@ -954,8 +958,9 @@ describe('errors', () => { expect(lexer.col).toBe(1) expect(lexer.formatError(undefined, "oh no!")).toBe( "oh no! at line 2 col 1:\n\n" + - " def quxx\n" + - " ^" + "1 abc\n" + + "2 def quxx\n" + + " ⬆︎\n" ) }) From 5b7314029a8415c6389c64ee45eb434fa4f82b60 Mon Sep 17 00:00:00 2001 From: Toby Ho Date: Fri, 9 Aug 2019 12:01:12 -0400 Subject: [PATCH 2/3] Optimized getting few lines of input to display. Revert arrow character to ^. Error handling in pad function Revert attaching of invalid token to error. --- moo.js | 74 +++++++++++++++++++++++++++++++++++------------ test/test.js | 82 +++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 124 insertions(+), 32 deletions(-) diff --git a/moo.js b/moo.js index 77041cd..80aa519 100644 --- a/moo.js +++ b/moo.js @@ -53,6 +53,39 @@ } } + function pad(n, length) { + var s = String(n) + if (s.length > length) { + return n + } + return Array(length - s.length + 1).join(" ") + s + } + + function lastNLines(string, numLines) { + var position = string.length + var lineBreaks = 0; + while (true) { + var idx = string.lastIndexOf("\n", position - 1) + if (idx === -1) { + break; + } else { + lineBreaks++ + } + position = idx + if (lineBreaks === numLines) { + break; + } + if (position === 0) { + break; + } + } + var startPosition = + lineBreaks < numLines ? + 0 : + position + 1 + return string.substring(startPosition).split("\n") + } + function objectToRules(object) { var keys = Object.getOwnPropertyNames(object) var result = [] @@ -531,7 +564,6 @@ // throw, if no rule with {error: true} if (group.shouldThrow) { var err = new Error(this.formatError(token, "invalid syntax")) - err.token = token throw err; } @@ -561,6 +593,8 @@ } } + Lexer.prototype.lastNLines = lastNLines + Lexer.prototype.formatError = function(token, message) { if (token == null) { // An undefined token indicates EOF @@ -574,25 +608,27 @@ } } - var lines = this.buffer.split("\n") - .slice( - Math.max(0, token.line - 5), - token.line - ); - var lastLineDigits = String(token.line).length - message += " at line " + token.line + " col " + token.col + ":\n\n" - message += lines - .map(function(line, i) { - return pad(token.line - lines.length + i + 1, lastLineDigits) + " " + line; - }) - .join("\n") - message += "\n" + pad("", lastLineDigits + token.col) + "⬆︎\n" - return message - - function pad(n, length) { - var s = String(n) - return Array(length - s.length + 1).join(" ") + s + var numLinesAround = 2 + var firstDisplayedLine = Math.max(token.line - numLinesAround, 1) + var lastDisplayedLine = token.line + numLinesAround + var lastLineDigits = String(lastDisplayedLine).length + var displayedLines = lastNLines( + this.buffer, + (this.line - token.line) + numLinesAround + 1 + ) + .slice(0, 5) + var errorLines = [] + errorLines.push(message + " at line " + token.line + " col " + token.col + ":") + errorLines.push("") + for (var i = 0; i < displayedLines.length; i++) { + var line = displayedLines[i] + var lineNo = firstDisplayedLine + i + errorLines.push(pad(lineNo, lastLineDigits) + " " + line); + if (lineNo === token.line) { + errorLines.push(pad("", lastLineDigits + token.col + 1) + "^") + } } + return errorLines.join("\n") } Lexer.prototype.clone = function() { diff --git a/test/test.js b/test/test.js index dd94af3..2cbe888 100644 --- a/test/test.js +++ b/test/test.js @@ -905,9 +905,9 @@ describe('errors', () => { expect(lexer.next()).toMatchObject({value: '456'}) expect(() => lexer.next()).toThrow( "invalid syntax at line 2 col 4:\n\n" + - "1 123\n" + - "2 456baa\n" + - " ⬆︎" + "1 123\n" + + "2 456baa\n" + + " ^" ) }) @@ -922,10 +922,12 @@ describe('errors', () => { expect(tok).toMatchObject({type: 'error', value: ' 12\n345\n6', lineBreaks: 2}) expect(lexer.formatError(tok, "numbers!")).toBe( "numbers! at line 3 col 2:\n\n" + - "1 abc\n" + - "2 def\n" + - "3 g 12\n" + - " ⬆︎\n" + "1 abc\n" + + "2 def\n" + + "3 g 12\n" + + " ^\n" + + "4 345\n" + + "5 6" ) }) @@ -940,9 +942,9 @@ describe('errors', () => { expect(lexer.col).toBe(9) expect(lexer.formatError(undefined, "EOF!")).toBe( "EOF! at line 2 col 9:\n\n" + - "1 abc\n" + - "2 def quxx\n" + - " ⬆︎\n" + "1 abc\n" + + "2 def quxx\n" + + " ^" ) }) @@ -958,9 +960,10 @@ describe('errors', () => { expect(lexer.col).toBe(1) expect(lexer.formatError(undefined, "oh no!")).toBe( "oh no! at line 2 col 1:\n\n" + - "1 abc\n" + - "2 def quxx\n" + - " ⬆︎\n" + "1 abc\n" + + "2 def quxx\n" + + " ^\n" + + "3 bar" ) }) @@ -1258,3 +1261,56 @@ describe('unicode flag', () => { }) }) + +describe("lastNLines", () => { + const lexer = moo.compile({}) + + test("grabs last n lines if more than needed", () => { + const input = "line 1\nline 2\nline 3" + expect(lexer.lastNLines(input, 2)) + .toEqual([ + "line 2", + "line 3" + ]) + }) + + test("grabs all lines if not enough", () => { + const input = "line 1\nline 2\nline 3" + expect(lexer.lastNLines(input, 10)) + .toEqual([ + "line 1", + "line 2", + "line 3" + ]) + }) + + test("grabs last empty line", () => { + const input = "line 1\nline 2\nline 3\n" + expect(lexer.lastNLines(input, 2)) + .toEqual([ + "line 3", + "" + ]) + }) + + test("grabs first empty line", () => { + const input = "\nline 1\nline 2\nline 3" + expect(lexer.lastNLines(input, 10)) + .toEqual([ + "", + "line 1", + "line 2", + "line 3" + ]) + }) + + test("does not grab first empty line if unneeded", () => { + const input = "\nline 1\nline 2\nline 3" + expect(lexer.lastNLines(input, 3)) + .toEqual([ + "line 1", + "line 2", + "line 3" + ]) + }) +}) From 4d94b61825065705082c5e403b78c8caa11b60c3 Mon Sep 17 00:00:00 2001 From: Toby Ho Date: Tue, 20 Aug 2019 21:22:38 -0400 Subject: [PATCH 3/3] Addressed code review comments. --- moo.js | 9 +++------ test/test.js | 53 ---------------------------------------------------- 2 files changed, 3 insertions(+), 59 deletions(-) diff --git a/moo.js b/moo.js index 80aa519..3552200 100644 --- a/moo.js +++ b/moo.js @@ -53,10 +53,9 @@ } } - function pad(n, length) { - var s = String(n) + function pad(s, length) { if (s.length > length) { - return n + return s } return Array(length - s.length + 1).join(" ") + s } @@ -593,8 +592,6 @@ } } - Lexer.prototype.lastNLines = lastNLines - Lexer.prototype.formatError = function(token, message) { if (token == null) { // An undefined token indicates EOF @@ -623,7 +620,7 @@ for (var i = 0; i < displayedLines.length; i++) { var line = displayedLines[i] var lineNo = firstDisplayedLine + i - errorLines.push(pad(lineNo, lastLineDigits) + " " + line); + errorLines.push(pad(String(lineNo), lastLineDigits) + " " + line); if (lineNo === token.line) { errorLines.push(pad("", lastLineDigits + token.col + 1) + "^") } diff --git a/test/test.js b/test/test.js index 2cbe888..f3300b0 100644 --- a/test/test.js +++ b/test/test.js @@ -1261,56 +1261,3 @@ describe('unicode flag', () => { }) }) - -describe("lastNLines", () => { - const lexer = moo.compile({}) - - test("grabs last n lines if more than needed", () => { - const input = "line 1\nline 2\nline 3" - expect(lexer.lastNLines(input, 2)) - .toEqual([ - "line 2", - "line 3" - ]) - }) - - test("grabs all lines if not enough", () => { - const input = "line 1\nline 2\nline 3" - expect(lexer.lastNLines(input, 10)) - .toEqual([ - "line 1", - "line 2", - "line 3" - ]) - }) - - test("grabs last empty line", () => { - const input = "line 1\nline 2\nline 3\n" - expect(lexer.lastNLines(input, 2)) - .toEqual([ - "line 3", - "" - ]) - }) - - test("grabs first empty line", () => { - const input = "\nline 1\nline 2\nline 3" - expect(lexer.lastNLines(input, 10)) - .toEqual([ - "", - "line 1", - "line 2", - "line 3" - ]) - }) - - test("does not grab first empty line if unneeded", () => { - const input = "\nline 1\nline 2\nline 3" - expect(lexer.lastNLines(input, 3)) - .toEqual([ - "line 1", - "line 2", - "line 3" - ]) - }) -})