Skip to content

Revert "Correctly parse comments in doctype declarations" #5591

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

Merged
merged 1 commit into from
Jun 10, 2025

Conversation

greg-at-moderne
Copy link
Contributor

Reverts #5581
as it's causing downstream problems, e.g.

/home/runner/work/rewrite-java-security/rewrite-java-security/src/main/java/org/openrewrite/java/security/xml/ExternalDTDAccumulator.java:32: error: incompatible types: Content cannot be converted to Element
                    for (Xml.Element element : docTypeDecl.getExternalSubsets().getElements()) {

Also this comment:
#5581 (comment)
needs to be addressed.

@MBoegers
Copy link
Contributor

We should defnetly have a second look if we could find a way to do both ;)

@MBoegers MBoegers marked this pull request as ready for review June 10, 2025 11:30
@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Jun 10, 2025
@greg-at-moderne
Copy link
Contributor Author

FYI @jhl221123

@greg-at-moderne greg-at-moderne merged commit 55a3991 into main Jun 10, 2025
2 checks passed
@greg-at-moderne greg-at-moderne deleted the revert-5581-fix-xml-parser-comment branch June 10, 2025 12:02
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Jun 10, 2025
@jhl221123
Copy link
Contributor

@greg-at-moderne, Thanks for the heads-up and for handling the revert so quickly. My apologies that my change caused this downstream issue; Please let me know if there's anything I can do to assist with the new approach.

JohannisK pushed a commit that referenced this pull request Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants