TargetNamespace.1/2 error range fix #761

datho7561 · 2020-06-03T19:08:08Z

If the declared namespace of the root element is not the same as
the namespace declared in the schema, the xmlns attribute of the root
element is highlighted.

If there is no namespace declared in the root element, but one is
declared in the schema, the root element is highlighted.

Added a CodeAction attached to TargetNamespace.1 that replaces
the namespace declared in the instance with the namespace declared in
the schema.

Added a CodeAction attached to TargetNamespace.2 that adds the
xmlns attribute to the root element and fills it with the namespace
used in the schema.

Fixes #704, Fixes #703

Signed-off-by: David Thompson [email protected]

fbricon · 2020-06-03T19:35:01Z

See if you can fix #703 too, as part of this PR

datho7561 · 2020-06-03T20:24:34Z

@fbricon I'll add a fix for it as well. Should I include the code snippets that were originally mentioned in this PR, or open a separate issue to fix them?

fbricon · 2020-06-03T20:40:16Z

Huh which snippets are you referring to?

datho7561 · 2020-06-03T21:04:55Z

@fbricon Oops. Sorry, I meant CodeActions. Angelo mentioned adding CodeActions in the issues. Should I make separate issues for these, or add them in this PR?

fbricon · 2020-06-04T00:06:50Z

We'll add code actions separately, in 0.13.0 or later

fbricon · 2020-06-04T08:50:18Z

I see you already have 1 code action. We should then have the other one as well, to be consistent. Hopefully it should be fairly straightforward to add now

datho7561 · 2020-06-04T15:10:27Z

Still working on the tests for them, but here are demos of the CodeActions:

angelozerr · 2020-06-04T17:38:56Z

Nice demo!

...se/lemminx/extensions/contentmodel/participants/codeactions/TargetNamespace_1CodeAction.java

...se/lemminx/extensions/contentmodel/participants/codeactions/TargetNamespace_2CodeAction.java

datho7561 · 2020-06-04T18:59:47Z

Nice demo!

Thank you!

...minx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSyntaxDiagnosticsTest.java

org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java

angelozerr · 2020-06-05T14:34:08Z

@datho7561 please fix the conflict

datho7561 · 2020-06-05T15:00:05Z

@datho7561 please fix the conflict

Fixed

xorye · 2020-06-05T15:02:22Z

...se/lemminx/extensions/contentmodel/participants/codeactions/TargetNamespace_1CodeAction.java

+		// The error message has this form:
+		// TargetNamespace.1: Expecting namespace 'NaN', but the target namespace of the
+		// schema document is 'http://two-letter-name'.
+		Matcher nsMatcher = NAMESPACE_EXTRACTOR.matcher(diagnosticMessage);


My worry is what if the namespace contains a ' (not url encoded like %XX) character?

@angelozerr do we need to worry about this?

Good catch. I looked at the specification to see if ' is allowed in a namespace: https://www.w3.org/TR/xml/#NT-Name
My understanding from this document is that it isn't.

Xerces doesn't complain when you write targetNamespace="'", so I think we should take care of even if it's not a common usecases.

Working on it and unit tests for it

I implemented it so that it will work with ' and escapes it when it occurs, but it doesn't recognize the escaped version of ', so it still raises TargetNamespace.1:

I've pushed the code that implements the above (with tests) into this PR. Right now, it always tries to escape single quotes. I left it as a separate commit in case it we want to revert it. Other options could be:

Always use double quotes when declaring xmlns if there is a single quote embedded in the targetNamespace

Don't give a code action if single quotes are prefered and there are single quotes in the targetNamespace

^This has been reverted back to the code that doesn't deal with single quotes in the namespace

...se/lemminx/extensions/contentmodel/participants/codeactions/TargetNamespace_1CodeAction.java

datho7561 · 2020-06-05T17:55:56Z

There are some issues with the build that I don't seem to have on my computer.

datho7561 · 2020-06-05T18:23:05Z

I ran the tests under a fresh Fedora 32 VM with openjdk 11, using the included maven wrapper and there were no failed tests.

angelozerr · 2020-06-05T19:02:47Z

If you repush your branch with amend (to restart the CI build) is it working better?

...c/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSchemaErrorCode.java

datho7561 · 2020-06-09T14:18:37Z

I've reverted my changes that try to resolve single quotes in the namespace. This PR is ready for review.

If the declared namespace of the root element is not the same as the namespace declared in the schema, the `xmlns` attribute of the root element is highlighted. If there is no namespace declared in the root element, but one is declared in the schema, the root element is highlighted. Added a CodeAction attached to `TargetNamespace.1` that replaces the namespace declared in the instance with the namespace declared in the schema. Added a CodeAction attached to `TargetNamespace.2` that adds the `xmlns` attribute to the root element and fills it with the namespace used in the schema. Fixes eclipse-lemminx#704, Fixes eclipse-lemminx#703 Signed-off-by: David Thompson <[email protected]>

fbricon requested a review from xorye June 3, 2020 19:33

datho7561 force-pushed the error-range-target-namespace-1 branch from a70fec4 to 9caf36d Compare June 3, 2020 20:22

datho7561 force-pushed the error-range-target-namespace-1 branch 2 times, most recently from 1150fea to bc46add Compare June 3, 2020 20:55

datho7561 changed the title ~~TargetNamespace.1 error highlights invalid xmlns~~ TargetNamespace.1/2 error range fix Jun 3, 2020

datho7561 force-pushed the error-range-target-namespace-1 branch from bc46add to 676c7d7 Compare June 3, 2020 21:11

datho7561 force-pushed the error-range-target-namespace-1 branch 2 times, most recently from ea81d9b to 591f66e Compare June 4, 2020 13:16

datho7561 force-pushed the error-range-target-namespace-1 branch 2 times, most recently from f3a2451 to 39d5806 Compare June 4, 2020 15:28

datho7561 marked this pull request as ready for review June 4, 2020 15:28

datho7561 force-pushed the error-range-target-namespace-1 branch from 39d5806 to 1ec1702 Compare June 4, 2020 15:30

angelozerr reviewed Jun 4, 2020

View reviewed changes

...se/lemminx/extensions/contentmodel/participants/codeactions/TargetNamespace_1CodeAction.java Show resolved Hide resolved

angelozerr reviewed Jun 4, 2020

View reviewed changes

...se/lemminx/extensions/contentmodel/participants/codeactions/TargetNamespace_1CodeAction.java Outdated Show resolved Hide resolved

angelozerr reviewed Jun 4, 2020

View reviewed changes

...se/lemminx/extensions/contentmodel/participants/codeactions/TargetNamespace_2CodeAction.java Show resolved Hide resolved

datho7561 force-pushed the error-range-target-namespace-1 branch from 1ec1702 to fc7ac7b Compare June 4, 2020 18:58

datho7561 force-pushed the error-range-target-namespace-1 branch from fc7ac7b to c7f9a97 Compare June 4, 2020 19:05

angelozerr reviewed Jun 5, 2020

View reviewed changes

...minx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSyntaxDiagnosticsTest.java Outdated Show resolved Hide resolved

angelozerr reviewed Jun 5, 2020

View reviewed changes

org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java Show resolved Hide resolved

datho7561 force-pushed the error-range-target-namespace-1 branch from c7f9a97 to db59752 Compare June 5, 2020 14:19

datho7561 force-pushed the error-range-target-namespace-1 branch from db59752 to d5893e0 Compare June 5, 2020 14:58

xorye reviewed Jun 5, 2020

View reviewed changes

datho7561 force-pushed the error-range-target-namespace-1 branch from d5893e0 to ea06c6f Compare June 5, 2020 15:23

fbricon reviewed Jun 5, 2020

View reviewed changes

...se/lemminx/extensions/contentmodel/participants/codeactions/TargetNamespace_1CodeAction.java Show resolved Hide resolved

datho7561 force-pushed the error-range-target-namespace-1 branch 2 times, most recently from 72f8e87 to 1adb388 Compare June 5, 2020 17:11

datho7561 force-pushed the error-range-target-namespace-1 branch from 1adb388 to a68a567 Compare June 5, 2020 19:11

angelozerr reviewed Jun 8, 2020

View reviewed changes

...c/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSchemaErrorCode.java Outdated Show resolved Hide resolved

datho7561 force-pushed the error-range-target-namespace-1 branch 2 times, most recently from f2dfe80 to c30cd98 Compare June 9, 2020 14:18

datho7561 force-pushed the error-range-target-namespace-1 branch from c30cd98 to 6949126 Compare June 9, 2020 14:34

xorye merged commit c1aeb8a into eclipse-lemminx:master Jun 9, 2020

datho7561 deleted the error-range-target-namespace-1 branch January 18, 2021 20:36

WolfgangHG mentioned this pull request Jan 3, 2022

Improve error message to show in the first line an understandable error message #1145

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TargetNamespace.1/2 error range fix #761

TargetNamespace.1/2 error range fix #761

datho7561 commented Jun 3, 2020 •

edited

Loading

fbricon commented Jun 3, 2020

datho7561 commented Jun 3, 2020

fbricon commented Jun 3, 2020 •

edited

Loading

datho7561 commented Jun 3, 2020

fbricon commented Jun 4, 2020

fbricon commented Jun 4, 2020

datho7561 commented Jun 4, 2020

angelozerr commented Jun 4, 2020

datho7561 commented Jun 4, 2020

angelozerr commented Jun 5, 2020

datho7561 commented Jun 5, 2020 •

edited

Loading

xorye Jun 5, 2020

datho7561 Jun 5, 2020

angelozerr Jun 5, 2020

datho7561 Jun 5, 2020

datho7561 Jun 8, 2020

datho7561 Jun 8, 2020

datho7561 Jun 9, 2020

datho7561 commented Jun 5, 2020

datho7561 commented Jun 5, 2020

angelozerr commented Jun 5, 2020

datho7561 commented Jun 9, 2020

TargetNamespace.1/2 error range fix #761

TargetNamespace.1/2 error range fix #761

Conversation

datho7561 commented Jun 3, 2020 • edited Loading

fbricon commented Jun 3, 2020

datho7561 commented Jun 3, 2020

fbricon commented Jun 3, 2020 • edited Loading

datho7561 commented Jun 3, 2020

fbricon commented Jun 4, 2020

fbricon commented Jun 4, 2020

datho7561 commented Jun 4, 2020

angelozerr commented Jun 4, 2020

datho7561 commented Jun 4, 2020

angelozerr commented Jun 5, 2020

datho7561 commented Jun 5, 2020 • edited Loading

xorye Jun 5, 2020

Choose a reason for hiding this comment

datho7561 Jun 5, 2020

Choose a reason for hiding this comment

angelozerr Jun 5, 2020

Choose a reason for hiding this comment

datho7561 Jun 5, 2020

Choose a reason for hiding this comment

datho7561 Jun 8, 2020

Choose a reason for hiding this comment

datho7561 Jun 8, 2020

Choose a reason for hiding this comment

datho7561 Jun 9, 2020

Choose a reason for hiding this comment

datho7561 commented Jun 5, 2020

datho7561 commented Jun 5, 2020

angelozerr commented Jun 5, 2020

datho7561 commented Jun 9, 2020

datho7561 commented Jun 3, 2020 •

edited

Loading

fbricon commented Jun 3, 2020 •

edited

Loading

datho7561 commented Jun 5, 2020 •

edited

Loading