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

Improve ETagRequired error range #881

Closed
wants to merge 1 commit into from

Conversation

angelozerr
Copy link
Contributor

Improve ETagRequired error range

Fixes #876

Signed-off-by: azerr azerr@redhat.com

@angelozerr angelozerr force-pushed the ETagRequired branch 3 times, most recently from 4de695b to c41d538 Compare September 23, 2020 15:02
@angelozerr
Copy link
Contributor Author

angelozerr commented Sep 23, 2020

My code requires to be cleaned and I need to add some tests again, but I think we can start to play with it.

Basicly, this PR improves error range for diagnostic. For instance given this XML

<abc>
  <def/>

Before def was highlighted which is wrong and provided code action are wrong.. Today it highlights abc and provided code action to close abc.

Other case is :

<abc>
</def>

It higlights def although it should higlight abc. Th ePR fixes that and provides 2 code actions:

  • one to close abc
  • a second to rename def to abc.

When I fixed that I need to fix code action to close abc. There were a lot of bugs and I fixed (by adding new tests) and I support the eat of characters (ex : it fixes the issue #878)

To test correctly this PR, I suggest you do step by step. Ex :

write

<a

see if the error range is correct and code action too.

write

<a>

write

<a><b

write

<a><b>

etc...

@datho7561
Copy link
Contributor

<foo>
  <bar><bar></</bar>
</foo>

Apply quickfix -->

<foo>
  <bar><bar</bar></bar>
</foo>

Apply quickfix -->

<foo>
  <bar><bar</bar>></bar>
</foo>

This behaviour is slightly different from what happens on master (on master the quickfixes don't help either), but this should probably be fixed in a different PR.

@datho7561
Copy link
Contributor

datho7561 commented Sep 23, 2020

Something else I found:

<foo>
  <bar><bar></bar>
  </bar></</bar>
</foo>

Apply quickfix -->

<foo</foo></foo>

@angelozerr angelozerr force-pushed the ETagRequired branch 3 times, most recently from 2262ca1 to 3443530 Compare September 24, 2020 13:50
Fixes eclipse-lemminx#876

Signed-off-by: azerr <azerr@redhat.com>
@angelozerr
Copy link
Contributor Author

Abandoned for #897

@angelozerr angelozerr closed this Oct 1, 2020
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.

Improve ETagRequired error range
2 participants