-
Notifications
You must be signed in to change notification settings - Fork 93
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
CodeLens, References, Rename, Diagnostics support for XML references #1435
Conversation
4d4a3dd
to
0570f26
Compare
@datho7561 the code should be good now (I need just to comments some piece of code) and tests are written. I support too multiple attribites (when referenced is declared in an attribute with spaces like: <link ana="jos-syn:Root" target="#ssj1.1.1 #ssj1.1.1.t24"/> Here some suggestion how to test this PR. Declare this settings: "xml.references": [
{
"pattern": "**/*.xml",
"expressions": [
{
"prefix": "#",
"from": "@corresp",
"to": "@xml:id"
},
{
"prefix": "#",
"from": "@resp",
"to": "persName/@xml:id"
},
{
"prefix": "#",
"from": "link/@target",
"to": "@xml:id",
"multiple":true
}
]
}
] Open this TEI file https://github.com/clarinsi/TEI-schema/blob/master/tei_clarin_example.xml You will see that there are a lof references like Here a demo |
If I try to rename <web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"
version="3.1">
<servlet>
<servlet-name>comingsoon-asdf</servlet-name>
<servlet-class>mysite.server.ComingSoonServlet</servlet-class>
</servlet>
<servlet-mapping>
<servlet-name>comingsoon-asdf</servlet-name>
<url-pattern>/*</url-pattern>
</servlet-mapping>
</web-app> Then it instead gets renamed to I can also confirm that the hyphen isn't what's causing this issue. |
I tried the following for multiple attributes: {
"from": "@ref",
"to": "bbb/text()",
"multiple": true
}, with the following XML: <aaa ref="child1 child2 child3">
<bbb>child1</bbb>
<bbb>child2</bbb>
<bbb>child3</bbb>
</aaa> The CodeLens appear, but they are not clickable. Rename, go to definition, go to reference, highlights, and linked editing don't work for this case. |
If I have: {
"from": "bbb/text()",
"to": "ccc/text()",
"multiple": true
}, With the XML: <aaa>
<bbb>asdf</bbb>
<bbb>hjkl</bbb>
<ccc>hjkl</ccc>
<ccc>asdf</ccc>
</aaa> Then if I try to rename at the <aaa>
<bbb>as|df</bbb>
<bbb>hjkl</bbb>
<ccc>hjkl</ccc>
<ccc>asdf</ccc>
</aaa> It lets me, but nothing happens (neither |
If I have the following document, with the same reference configuration as with the previous comment, and I try to rename between the two spaces at the <aaa>
<bbb>asdf | hjkl</bbb>
<bbb>hjkl</bbb>
<ccc>hjkl</ccc>
<ccc>asdf</ccc>
</aaa> It allows me to attempt to rename it. Is this related to |
fa8f51b
to
ad009c0
Compare
32ea1aa
to
af06503
Compare
I think that another bug that you fixed also fixed this bug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, so I made it through the code. The only things left to be fixed are:
- the case where if you have just the prefix for a reference, you can open the rename popup
- this license header is slightly different from the one we normally use
After that, I think the PR is ready to be merged.
...t/java/org/eclipse/lemminx/extensions/references/XMLReferencesDiagnosticsExtensionsTest.java
Outdated
Show resolved
Hide resolved
7a9c3de
to
6e2c958
Compare
If you have the following XML: <aaa>
<bbb>| </bbb>
<ccc>asdf</ccc>
</aaa> with the following references setting: {
"from": "bbb/text()",
"to": "ccc/text()",
"multiple": true,
"prefix": "" // also happens with no "prefix" defined
}, Then, if you try to rename at the |
6e2c958
to
b2f42e0
Compare
It should be fixed (with test too) |
There's a test failure, but it looks like you just typed in the expected output incorrectly. |
b2f42e0
to
422cf67
Compare
|
You fixed that issue in one of the changes you made. It should work now. |
fcb9eb3
to
be9fef0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed something in my previous review: I think the prefix diagnostic wording could be improved
...g/eclipse/lemminx/extensions/references/participants/XMLReferencesDiagnosticParticipant.java
Show resolved
Hide resolved
...g/eclipse/lemminx/extensions/references/participants/XMLReferencesDiagnosticParticipant.java
Outdated
Show resolved
Hide resolved
544cee8
to
fcbc782
Compare
Signed-off-by: azerr <azerr@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for being patient with my review, Angelo.
Thanks so much for having taken so many time to test corner usescases and report so many bugs :) |
@oscarlevin I think this issue could interest you. If you write this settings "xml.references": [
{
"pattern": "**/*.ptx",
"expressions": [
{
"from": "xref/@ref",
"to": "@xml:id"
}
]
}
] You should benefit with all XML references support described in https://github.com/redhat-developer/vscode-xml/blob/main/docs/Features/XMLReferencesFeatures.md Here a demo with your ptx files: |
CodeLens, References, Rename, Diagnostics support for XML references
Signed-off-by: azerr azerr@redhat.com