Range in characters is sub-optimal for error reporting to humans

[MikeRalphson]

As YAML is line-delimited (can't be strung together minified like JSON) it makes at least some sense to be able to report errors (both syntactically and semantically) as a line number and column number pair.

Is it simple to add a helper routine to achieve this? Happy to PR with a bit of pointing in the right direction.

My end use-case is to turn JSON pointer references to schema violations back into line number, column number pairs.

[eemeli]

That sounds like a good idea, and a PR would indeed be welcome. This may need to expose two different APIs, e.g. something like charPosToLineCol(pos, str): { line: number, col: number } for the actual implementation (you may be able to find something already on npm) and a wrapper convenience method added to the errors (which due to this may need a common ancestor).

Things to be aware of:

• A Document or its parsed nodes do not directly contain any reference to the source string, unless keepCstNodes is enabled.
• Parsed AST nodes contain a range: [start, end] property; nodes not generated by parsing obviously will not.
• CST nodes always have a range: { start: number, end: number } property
• Errors always contain a source reference to their CST node, which in turn do contain a reference to the source string as context.src
• When parsing, if the input contains \r\n line terminators, those are converted to \n terminators before processing. The string at context.src will only have \n line terminators. The CST parser's ability to add origRange pointers to the real original source isn't (currently) available when using the library via e.g. YAML.parse() or YAML.parseDocument(), and should not be strictly required for this.

[MikeRalphson]

Ok thanks. I'm making some progress, able to build up a list of line offsets at the same point the \rs are removed in the CST parser which should allow a new charPosToLineCol function to be performant.

Will PR as soon as I have a tested solution. Please consider me working on this issue.

[eemeli]

Do keep in mind that adding a cost for each parser run might overall not be worth it, as it's relatively rare that you'll encounter an error, and then want to display it to a human in a friendly manner. At that point we've already failed, and spending a few extra cycles might make more sense than doing something on every run of the parser.

[MikeRalphson]

Ok, just to clarify, you mean it's only worth computing the line offsets and doing the conversion when explicitly asked for (i.e. the caller can re-parse the document(s) with some option flag turned on or parse at a lower level)?

In my use case (an OpenAPI validator and linter) errors are commonly encountered and almost expected, though I accept this may be an outlier.

[eemeli]

Correct. The parser doesn't otherwise need to track newlines, so we should not require that it does so every time. However, as you may get multiple errors from a single input document, it may well make sense to cache the positions of newlines when they're first determined.

[MikeRalphson]

Thanks. Will put the calculation behind an option.

[MikeRalphson]

Could you let me know how yaml-test-suite should be set up for all the tests to run to completion? Couldn't see anything in the README or previous issues.

[eemeli]

Drat, sorry, should add developer guidelines with that info. There's a git submodule that you need. Run this in the project root:

git submodule update --init

[MikeRalphson]

Perfect, thanks. Worked once I cleared out the basic clone I'd done into that directory.

[eemeli]

I don't know what's up with that error. That commit definitely exists.

[MikeRalphson]

Ignore that, I had a clone of YAML-test-suite in that directory I forgot to remove.

[MikeRalphson]

should add developer guidelines with that info.

Let me know if you want a PR for this, and if so, against README.md or a new CONTRIBUTING.md?