Add error tokens #25
Add error tokens #25
Conversation
|
@nathan @Hardmath123 This is kind of mad, but I thought it might be worth a try. :) |
moo.js
Outdated
| @@ -13,6 +13,8 @@ | |||
|
|
|||
| function isRegExp(o) { return o && o.constructor === RegExp } | |||
|
|
|||
| var ERROR = {error: true} | |||
| Object.freeze(ERROR) | |||
There was a problem hiding this comment.
It's seems nicer to write this as var ERROR = Object.freeze({error: true})
moo.js
Outdated
| @@ -73,7 +78,8 @@ | |||
| } else if (Array.isArray(obj)) { | |||
| // sort to help ensure longest match | |||
| var options = sortPatterns(obj) | |||
| return '(' + options.map(regexpOrLiteral).join('|') + ')' | |||
| options = options.map(regexpOrLiteral).filter(function(o) { return o !== ERROR }) | |||
There was a problem hiding this comment.
It might be cleaner return null or undefined from regexpOrLiteral and filter based on truthiness (filter(function(o) {return o})). I'm not sure
moo.js
Outdated
| @@ -116,6 +123,14 @@ | |||
| // convert to RegExp | |||
| var pat = pattern(obj.match) | |||
|
|
|||
| if (pat === ERROR || (Array.isArray(obj) && obj.match.indexOf(ERROR) !== -1)) { | |||
| if (error) { | |||
| throw new Error("Cannot have multiple error rules") | |||
There was a problem hiding this comment.
It's nice to provide context for the API user in the error message—the name of the token that generated this error, and perhaps the name of the token already marked as error as well. You should also add a compiler test case to ensure that this error is thrown when appropriate, and that the message contains the correct information.
moo.js
Outdated
| group = Lexer.error | ||
| group = this.error | ||
| if (!group) { | ||
| throw new Error('Syntax error') |
There was a problem hiding this comment.
This is fine for now, but perhaps we should think about making it easy for us/users to generate nice syntax errors—with line/column information and a preview of the buffer pointing to the problematic character, for example.
|
This API seems rather odd to me. Why not just: myError: { error: true }, // error should imply lineBreaks
// or optionally:
myError: { match: /[`$\t]/, error: true },This simplifies the implementation considerably and does less magic with the |
tjvr
force-pushed
the
errors
branch
2 times, most recently
from
7220b2a to
0399ea2
Compare
I did consider this. I liked that this more complicated way made a rule that matched a pattern and handled errors more obvious: e.g. Sorry about the rebase; #27 moved everything around, and since we're going with the simpler API it wasn't worth re-implementing it on top of #27! |
| @@ -380,9 +404,10 @@ | |||
| } | |||
There was a problem hiding this comment.
You still need to copy error in Lexer#clone. Actually, using assign({}, s, {regexp: …}) might be better so we don't have to keep updating it.
moo.js
Outdated
| continue | ||
| } | ||
| delete options.error | ||
| } |
There was a problem hiding this comment.
It seems like you don't have to split it into two rules as long as compileRules knows to skip over rules with no match. Not sure if this would be more or less clear.
There was a problem hiding this comment.
Unfortunately Nevermind!sortRules won't emit a rule with an empty match array. :-(
There was a problem hiding this comment.
Note that implies that rules with no error or match set would be silently skipped.
@nathan Feel free to refactor this to use assign() if you so wish. :-)
A proposed scheme for #18.
By default, if no token matches, moo will throw a syntax error.
Unless you define one of your tokens to match the special pattern
moo.error, in one of the following ways:...in which case, moo will instead return a token object with
type: myError, with the value being the remainder of the input buffer.