Pass the line number and context back with the validation results

RESOLVED FIXED in 5.12.3

Status

P1
normal
RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: clouserw, Assigned: basta)

Tracking

unspecified
5.12.3

Details

(Reporter)

Description

8 years ago
We've talked about this before, but just to make sure we don't forget:  we need to pass the line number the error happened on, and the context (140 chars on either side) back with the result JSON.
(Assignee)

Comment 1

8 years ago
This is implemented on the "contextgenerator" branch of the validator. In the JSON, the messages should have a "context" property that contains up to three strings. The middle string should be the line that the error was detected at. The other two are the surrounding lines.

Lines are truncated at 140 characters. Since position information is rarely, if ever, available, it is still difficult to handle very long lines (as in minified JS files). The JS traverser currently does not support position information; this is a planned future enhancement.

install.rdf files do not have contexts (rdflib does not pass back line numbers). This is an item for future development.

If someone could look over the code, that would be great.

https://github.com/mattbasta/amo-validator/blob/contextgenerator/validator/contextgenerator.py

Also, if there is a more useful format for the output in the JSON, please let me know.
(Reporter)

Comment 2

8 years ago
That's looking good.  We want the 140 chars trimmed to the closest side.  For example:

 ...some really long line
 the line that broke
 some other long line...


CCing Kumar who is doing the front end
(Reporter)

Comment 3

8 years ago
Oh, you wrote what I'm talking about in Bug 532789.  Is that the same file on a different branch?  We should use that one
(Assignee)

Comment 4

8 years ago
(In reply to comment #3)
> Oh, you wrote what I'm talking about in Bug 532789.  Is that the same file on a
> different branch?  We should use that one

It is, it's a more up-to-date version that includes the proper truncation stuff. That one is merged into master now.
(Assignee)

Comment 5

8 years ago
This should be done in 79ec446; there are now unit tests! Hooray!

https://github.com/mattbasta/amo-validator/commit/79ec4469854a6c373471a5e6d6c28b51d684e1ac
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.