Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a more sensible fallback range if there is no parent property #30

Closed
wants to merge 1 commit into from
Closed

Add a more sensible fallback range if there is no parent property #30

wants to merge 1 commit into from

Conversation

Levertion
Copy link
Contributor

@Levertion Levertion commented Oct 24, 2018

EDIT: After some consideration I realise this should be an issue rather than a pull request, as this is not obviously better in all cases.

Before:

{"bold":true}
^
Missing property "text"

After:

{"bold":true}
^^^^^^^^^^^^^
Missing property "text".

This case could occur if the entire document was validated against a schema using `required` or `oneOf`. This would have the disadvantage of making the errors uglier in cases where there is a large document which fails this validation, however only erroring the first character is also not great. Perhaps we should work out an alternative method?

@aeschli
Copy link
Contributor

aeschli commented Oct 24, 2018

Multi line squiggles or even the whole document being red is really ugly and that was the reason to make the error that small.
I'm not really in flavor of this change.

@Levertion
Copy link
Contributor Author

Yeah, that's entirely reasonable. My use-case is for another format with inline json support on one line, where the single space squiggle appears to be too small, but it becomes ugly for a larger string anyway.

There is no good solution, but I'll close this.

@Levertion
Copy link
Contributor Author

Would it be worth having an heurstic such as if the node length is less than, say 35 we give an error over the whole range, else give an error on the starting character - for smallish sections the small error range is somewhat unfortunate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants