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

Event::DocType skip initial whitespaces #325

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

BlueGreenMagick
Copy link
Contributor

@BlueGreenMagick BlueGreenMagick commented Oct 11, 2021

Document that the whitespace right after <!DOCTYPE is included when reading, and should be included when writing.

I personally think it may be better to exclude the first whitespace from DocType, but that might be a breaking change, so I added a line to documentation instead.

@Mingun
Copy link
Collaborator

Mingun commented Oct 11, 2021

I think it is better to fix such kinds of problems while we at the 0.x release cycle which is intended for such fixes. Breaking changes is allowed (and expected) between 0.x releases.

@tafia
Copy link
Owner

tafia commented Oct 19, 2021

I agree to change the behavior if people feel confident this is the best choice. I am not using DOCTYPE much and I am fine having few breaking changes because the next version is already breaking few things.
Best to mention it in the Changelog as well.

sample_4 test is not enabled yet because of CData
@BlueGreenMagick BlueGreenMagick changed the title Event::DocType document that first char is a space Event::DocType skip initial whitespaces Oct 25, 2021
@BlueGreenMagick
Copy link
Contributor Author

Done!

Changes:

  • When parsing, skips initial whitespaces after "!DocType". Creates BytesText("") if there is only only whitespace after !DocType even though it's illegal according to the docs.
  • When writing, put a single whitespace between !DocType and BytesText content.
  • Enabled test_doctype test for serialized feature.

@tafia tafia merged commit 521d72a into tafia:master Oct 26, 2021
@tafia
Copy link
Owner

tafia commented Oct 26, 2021

Thanks!

@BlueGreenMagick BlueGreenMagick deleted the doctype-whitespace branch October 31, 2021 12:56
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