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

Fix error range for ETagRequired #395

Merged
merged 1 commit into from
May 29, 2019
Merged

Fix error range for ETagRequired #395

merged 1 commit into from
May 29, 2019

Conversation

xorye
Copy link

@xorye xorye commented May 27, 2019

Fixes #387

image

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

@fbricon
Copy link
Contributor

fbricon commented May 27, 2019

Please use self-descriptive commit messages, like Fix error range for ETagRequired

@xorye
Copy link
Author

xorye commented May 27, 2019

Ok, I have just changed the commit message.

@fbricon
Copy link
Contributor

fbricon commented May 27, 2019

LGTM but I'll let @NikolasKomonen or @angelozerr check if that's actually correct.

@NikolasKomonen
Copy link
Contributor

@xorye Looks good to me

One thing to change, and I know you didn't do this. But could you update the method name of 'findChildNode' https://github.com/angelozerr/lsp4xml/blob/ccc55276ec3505f93a3d654235560f5e3504a03b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/utils/XMLPositionUtility.java#L205

to something like 'findUnclosedChildNodeWithName'

Other than than it works great.

@fbricon
Copy link
Contributor

fbricon commented May 29, 2019

One thing to change, and I know you didn't do this. But could you update the method name of 'findChildNode' to something like 'findUnclosedChildNodeWithName'

findUnclosedChildNode will be fine

@xorye
Copy link
Author

xorye commented May 29, 2019

Cool, changing it now.

@xorye
Copy link
Author

xorye commented May 29, 2019

Actually, this function finds a closed child node. Not an unclosed one.

I'll change it to findClosedChildNode

}
}
return null;
}

static DOMNode findChildNode(String childTag, List<DOMNode> children) {
static DOMNode findClosedChildNode(String childTag, List<DOMNode> children) {
for (DOMNode child : children) {
if (child.isElement() && childTag != null && childTag.equals(((DOMElement) child).getTagName())
&& !child.isClosed()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this child is unclosed

Fixes #387

Signed-off-by: David Kwon <[email protected]>
@xorye xorye merged commit c228469 into eclipse:master May 29, 2019
@xorye xorye deleted the errorRangeEtag branch May 29, 2019 14:45
@xorye xorye restored the errorRangeEtag branch May 29, 2019 18:57
NikolasKomonen added a commit that referenced this pull request May 29, 2019
angelozerr added a commit to angelozerr/lemminx that referenced this pull request Jun 3, 2021
angelozerr added a commit to angelozerr/lemminx that referenced this pull request Jun 3, 2021
angelozerr added a commit to angelozerr/lemminx that referenced this pull request Jun 3, 2021
angelozerr added a commit to angelozerr/lemminx that referenced this pull request Jun 3, 2021
angelozerr added a commit to angelozerr/lemminx that referenced this pull request Jun 3, 2021
angelozerr added a commit to angelozerr/lemminx that referenced this pull request Jun 4, 2021
angelozerr added a commit to angelozerr/lemminx that referenced this pull request Jun 4, 2021
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.

Fix error range for ETagRequired
4 participants