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

Provide better Error Validation messages. #181

Closed
NikolasKomonen opened this issue Oct 30, 2018 · 12 comments
Closed

Provide better Error Validation messages. #181

NikolasKomonen opened this issue Oct 30, 2018 · 12 comments
Assignees
Labels
enhancement New feature or request validation
Milestone

Comments

@NikolasKomonen
Copy link
Contributor

Improve the clarity of error validation messages.

@angelozerr
Copy link
Contributor

What do you mean with clarity? Error message comes from Xerces, you wish to override it?

@NikolasKomonen
Copy link
Contributor Author

@fbricon yes, for example

<project xmlns="http://maven.apache.org/POM/4.0.0" 
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
	<modelVersion>4.0.0</modelVersion>
	...
        <a></a>
        ...

Gives the output:

[xml] cvc-complex-type.2.4.a: Invalid content was found starting with element '{"http://maven.apache.org/POM/4.0.0":a}'. One of '{"http://maven.apache.org/POM/4.0.0":parent, "http://maven.apache.org/POM/4.0.0":name, "http://maven.apache.org/POM/4.0.0":description, "http://maven.apache.org/POM/4.0.0":url, "http://maven.apache.org/POM/4.0.0":inceptionYear, "http://maven.apache.org/POM/4.0.0":organization, "http://maven.apache.org/POM/4.0.0":licenses, "http://maven.apache.org/POM/4.0.0":developers, "http://maven.apache.org/POM/4.0.0":contributors, "http://maven.apache.org/POM/4.0.0":mailingLists, "http://maven.apache.org/POM/4.0.0":prerequisites, "http://maven.apache.org/POM/4.0.0":modules, "http://maven.apache.org/POM/4.0.0":scm, "http://maven.apache.org/POM/4.0.0":issueManagement, "http://maven.apache.org/POM/4.0.0":ciManagement, "http://maven.apache.org/POM/4.0.0":distributionManagement, "http://maven.apache.org/POM/4.0.0":properties, "http://maven.apache.org/POM/4.0.0":dependencyManagement, "http://maven.apache.org/POM/4.0.0":dependencies, "http://maven.apache.org/POM/4.0.0":repositories, "http://maven.apache.org/POM/4.0.0":pluginRepositories, "http://maven.apache.org/POM/4.0.0":build, "http://maven.apache.org/POM/4.0.0":reports, "http://maven.apache.org/POM/4.0.0":reporting, "http://maven.apache.org/POM/4.0.0":profiles}' is expected.

which I feel can be simplified

@angelozerr
Copy link
Contributor

I understand your idea, but I'm not sure that's a good idea to override messages. Those messages comes from Xerces and you can use google to understand more your problem, because Xerces is used in an so many project like IJ IDE, Eclipse, etc.

@fbricon what do you think about this issue?

@fbricon
Copy link
Contributor

fbricon commented Oct 30, 2018

From a user standpoint, those messages are not super useful (dare I say horrible?) as they're very hard to parse.
From a technical standpoint, I understand it can be tricky to override.
Now let's keep that issue open for now, as we have other stuff to fix first, but anyone having an idea on how to make it right can chime in.

@akaroml
Copy link

akaroml commented Nov 20, 2018

Not familiar with Xerces but can it provide error message in the format of structured data?

@NikolasKomonen
Copy link
Contributor Author

NikolasKomonen commented Nov 20, 2018

Here is the property file where the messages for schemas are stored in Xerces.

This is where Xerces assigns the resource path.

Here is where lsp4xml assigns Xerces' XSMessageFormatter, which I assume we can override to point to a modified property file with better messages.

@angelozerr correct me if I'm wrong.

@NikolasKomonen NikolasKomonen changed the title Make better Error Validation messages. Provide better Error Validation messages. Nov 20, 2018
@akaroml
Copy link

akaroml commented Nov 21, 2018

Here is the property file where the messages for schemas are stored in Xerces.

This is where Xerces assigns the resource path.

Here is where lsp4xml assigns Xerces' XSMessageFormatter, which I assume we can override to point to a modified property file with better messages.

@angelozerr correct me if I'm wrong.

This is so specific. Thank you for the info.

I have to echo this issue since it's so painful to look at the error info in VS Code. We'll be more confident to recommend this extension to our users with this problem addressed.

@angelozerr
Copy link
Contributor

Here is where lsp4xml assigns Xerces' XSMessageFormatter, which I assume we can override to point to a modified property file with better messages.

I had to do that because I lost Xerces messages. I don't know if it's a clean solution.

To be honnest with you, I have no time to work on this issue. @fbricon @NikolasKomonen is it an important issue for you? Have you planned to work on this issue?

@NikolasKomonen
Copy link
Contributor Author

@angelozerr I am able to work on this issue after my part for DTD issues.

@fbricon
Copy link
Contributor

fbricon commented Nov 21, 2018

There's a ton of possible error messages to override. I suggest we focus on the 5-10 most common ones for starters, see where it goes.

@fbricon fbricon added this to the v0.0.4 milestone Nov 21, 2018
@fbricon
Copy link
Contributor

fbricon commented Nov 21, 2018

I have to echo this issue since it's so painful to look at the error info in VS Code. We'll be more confident to recommend this extension to our users with this problem addressed.

@akaroml you'll get the exact same error messages from Eclipse or IntelliJ so users should not be surprised here. Is it ok? Not really, but at least we're on the same playing field. You should also consider the other features brought to the table when considering recommending the Red Hat's vscode-xml extension.

@NikolasKomonen
Copy link
Contributor Author

If anyone finds messages they would like to be simplified please post them here.

@NikolasKomonen NikolasKomonen self-assigned this Nov 27, 2018
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 31, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 7, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 7, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 8, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 11, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 11, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 11, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 20, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 25, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 25, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 26, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Feb 28, 2019
Fixes eclipse-lemminx#181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
fbricon pushed a commit that referenced this issue Mar 1, 2019
Fixes #181

Signed-off-by: Nikolas <nikolaskomonen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request validation
Projects
None yet
Development

No branches or pull requests

4 participants