Error message method

[JonathanMEdwards]

I'd like to report syntax errors with line and column info. But ohm does not export common.getLineAndColumnMessage. I'd like to write something like the following function. Or even better it should be a method on Interval.

export function syntaxError(interval: ohm.Interval, message: string): void {
    throw new Error(
        ohm.getLineAndColumnMessage(
            interval.inputStream.source,
            interval.startIdx) +
        message);
}

[alexwarth]

You're right, this kind of thing should be easy to do. And I think it is in this case: have you tried the message and shortMessage properties of the MatchFailure object that you get from a failed call to yourGrammar.match(...)? Let me know if those properties don't do what you want and I'll figure something out :)

Alternatively, you could compose your own error message using a combination of other methods from MatchFailure. It's more work, but gives you more control:

• getPos() method will tell you where the match failed. (We should probably also support a getLineAndCol method, so that clients don't have to compute that information themselves.)
• getExpectedText() gives you a string, e.g., "+, -, *, or /"

[JonathanMEdwards]

Sorry - this is for when the parse is succeeding but I want to report errors detected later in the semantics.

[alexwarth]

Oh, I see! Thanks for clarifying.

I think your suggestion makes perfect sense. I can imagine exposing this kind of functionality in a couple of different ways. One option (as you suggested) would be to add it as a method on Intervals. Another would be to make it a method of CST nodes (e.g., someNode.createErrorMessage('a string that explains what is wrong with this node')) -- then you wouldn't even have to extract the node's interval. IMO the latter would be nicer to use, assuming it's expressive enough. Can you think of any cases where you'd want to create a custom interval?

Anyway, I'll leave this one for Pat to look at on Monday. He's been working on improving the way error messages are displayed, so it's probably best if he adds this feature.

Thanks again, Jonathan!

[JonathanMEdwards]

Great, thanks. I'd vote for a method on Interval, because it only depends on info from the Interval. I've been stashing Intervals in the DOM because that's all i need so far.

Another reason I haven't been using Nodes is because semantic actions get the current node as a dynamic this, which is subtle and not expressible in the Typescript type system and not available to ES6 lambda expressions.

[pdubroy]

As of d376cf3, it's exposed via ohm.util.getLineAndColumnMessage. I will probably also make a convenience method on Interval, but I thought it would be better to have a more flexible version for cases for when you might not have an interval in hand.

[pdubroy]

@JonathanMEdwards, would you mind opening a new issue with some more details about the problem with dynamic this in Typescript? We'll eventually want to figure out a solution that works well in other languages.

[JonathanMEdwards]

Upon reflection I think the current design is the best. It is certainly
idiomatic javascript. Dynamic this and first-class methods are outliers in
the landscape of OO languages but it actually works nicely in this case.
Typescript doesn't allow dynamic this to be typed but still allows it to be
used.

On Mon, Jun 1, 2015 at 6:36 AM, Patrick Dubroy notifications@github.com
wrote:

@JonathanMEdwards https://github.com/JonathanMEdwards, would you mind
opening a new issue with some more details about the problem with dynamic
this in Typescript? We'll eventually want to figure out a solution that
works well in other languages.

—
Reply to this email directly or view it on GitHub
#15 (comment).