keymanapp
feat(developer): support line numbers for LDML compiler errors
Our messages (errors, hints, and warnings) don't have line numbers at present, which is going to be quite painful for users!
Originally posted by @mcdurdin in https://github.com/keymanapp/keyman/issues/10614#issuecomment-1926061336
Sorry, I was in file mode and didn't see this:
per #10614 quoth @mcdurdin
Our messages (errors, hints, and warnings) don't have line numbers at present, which is going to be quite painful for users!
right, I'm not sure how we can get that out of the xml parser though.
What I did for CLDR is have the parser record a xpath-to-line number mapping, with a custom SAX parser. If we had something similar here (.../key[@id="hamza"] or .../transformGroup[@3]/transform[@2] for 'the second transform in the third group') could be collected in the CompilerEvent, and then (probably) re-parse the XML (expanding imports!) looking for something with a match.
My point is that where the error actually occurs there isn't any context of the actual XML, but it might have enough section-relative data to be able to construct an xpath.
The slightly-related topic here is that we could also deduplicate errors at that point as well, particularly errors on the same part of the XML. "This key has these errors.."
What do you think?
right, I'm not sure how we can get that out of the xml parser though.
Relates to https://github.com/keymanapp/keyman/issues/8616#issuecomment-1630013900 and choices we make in the future. Sounds like maintaining line/char position could be a required feature for our xml parser of choice?
My point is that where the error actually occurs there isn't any context of the actual XML, but it might have enough section-relative data to be able to construct an xpath.
So tracking context throughout the compiler would also be helpful -- whether we do that by xpath or by line/char, either will work.
This is also going to be useful for interactive debugging, assuming we go there (which would be a new section in the KMXPlus format).
Perhaps we could use JavaScript symbols for the Line information, similar to what I'm already doing for import information
CompilerEvent already has:
export interface CompilerEvent {
filename?: string;
line?: number;
code: number;
message: string;
/**
* an internal error occurred that should be captured with a stack trace
* e.g. to the Keyman sentry instance by kmc
*/
exceptionVar?: any;
};
filename/line aren't currently used, at least by ldml. I kind of wonder if they should be an array, that way there could be multiple lines/files with the same error? But, that could be coalesced later.
Also an xpath or something could go into this struct.
filename/line aren't currently used, at least by ldml
They are used by the other compilers though!
I like the simplicity of this structure as it is. Adding an xpath could be useful but xpath needs to be translated back to file+line for editor use anyway.
#10733 is a dup. Example of unhelpful message:
ldml_test.xml: Error: 0007 The source file has an invalid structure: Unable to load XML file
See comments on that issue for additional work here.
