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

DTD losing content on format fixed #200

Merged
merged 1 commit into from
Nov 7, 2018

Conversation

NikolasKomonen
Copy link
Contributor

Fixes #198

@Test
public void testDTDFormattingInternal() throws BadLocationException {
String content =
"<?xml version=\"1.0\"?>" + lineSeparator() +
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't use lineSeparator() because your tests depends on OS. Please use \r or \ r\n.

If you use Eclipse to develop Java, you can copy a content of XML file and go to the Java Editor code. Type just "" and copy/paste the pasted XML content inside the "". Java Editor JDT will manage \r

Copy link
Contributor Author

@NikolasKomonen NikolasKomonen Nov 6, 2018

Choose a reason for hiding this comment

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

I though \r\n was specific to Windows, do you know why it works for me on linux when I run tests using \r\n?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry with my bad explanation.

I though \r\n was specific to Windows,

Yes it is.

I meant that we have tests which is not linked to an OS. So in your Linux case, you will havelineSparatator= \r and in my OS Windows case, I will have lineSparatator=\r\n So the offset will be different.

I agree with you, your test doesn't use offset so we will hav eno problem, but it should be more clean to have tests which hard coded the line separator.

Hope you will understand my explanation.

@angelozerr
Copy link
Contributor

Very cool PR @NikolasKomonen ! IMHO, I think it should really cool if we could use your scanner to manage in XMLDocument, the Entity to display it in the Outline, manage completion, etc

@NikolasKomonen
Copy link
Contributor Author

I think it should really cool if we could use your scanner to manage in XMLDocument, the Entity to display it in the Outline, manage completion, etc

@angelozerr yes! Thats what I was hoping we could eventually work on. This pr is just temporary to make sure DTD content doesn't disappear.

@angelozerr
Copy link
Contributor

@angelozerr yes! Thats what I was hoping we could eventually work on. This pr is just temporary to make sure DTD content doesn't disappear.

Great!

@fbricon
Copy link
Contributor

fbricon commented Nov 6, 2018

PR fixes the issues. @NikolasKomonen fix the line endings and we can merge it

@NikolasKomonen
Copy link
Contributor Author

@fbricon line endings fixed

@fbricon fbricon merged commit ca150a3 into eclipse-lemminx:master Nov 7, 2018
@NikolasKomonen NikolasKomonen deleted the dtdFormattingFix branch January 23, 2019 19:36
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.

3 participants