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

[header] Support header comments so line breaks properly #1664

Closed
wants to merge 1 commit into from

Conversation

hazendaz
Copy link
Contributor

@hazendaz hazendaz commented Nov 4, 2021

context: formatter-maven-plugin

Issue: Many files come with copyright headers in the files. The html parser for pretty printing with jsoup treats this header comment incorrectly and ends up adding the html tag at end of --> on same line. I'm not sure the fix I have here is appropriate but it did solve the issue.

As with before you can see this with this file here.

@hazendaz hazendaz marked this pull request as draft November 4, 2021 02:26
@hazendaz
Copy link
Contributor Author

hazendaz commented Nov 4, 2021

For now looking for feedback only. Depending on feedback will adjust tests as needed.

@hazendaz hazendaz marked this pull request as ready for review November 25, 2021 01:31
@hazendaz
Copy link
Contributor Author

This is ready for review. Basic idea here is license header comments used in html pages. The current jsoup will not line break after the comment thus the block start is at end of the header comment which is not correct. To avoid touching other comments, this addresses only when 'accum' is empty and thus start of the file processing when pretty print is turned on. An existing toString test already runs full pretty print and was able to be adjusted to confirm this test while keeping all other existing tests intact.

@hazendaz
Copy link
Contributor Author

@jhy Could you look at this one?

@hazendaz
Copy link
Contributor Author

You can see this saved item Saved https://try.jsoup.org/~pWWgM1Tq8l3YM1WTm8Fftq5ziv4, look at the header as it buts up against the docttype. It should line break which this one attempts to fix and does work.

…print

License headers will be comments at start of html.  Pretty print today does not line break
when it writes it thus it is not properly pretty printed.  This patch looks for usage of
pretty print and fact that 'accum' is empty to ensure the proper line break.
@hazendaz
Copy link
Contributor Author

hazendaz commented Jan 3, 2022

rebased, still not fixed until this commit

@hazendaz
Copy link
Contributor Author

hazendaz commented Jan 9, 2022

Jsoup Header formatting issue

Attached is after running jsoup pretty format. Typically license header will be at top of the file and it fails to properly account for that. See the <!doctype> is in end of the comment to header. This fixes it issue.

@jhy jhy closed this in 67b48dd Jun 19, 2022
@jhy
Copy link
Owner

jhy commented Jun 19, 2022

Thanks @hazendaz - I've fixed this now with a bit of a different approach. See 67b48dd - when pretty-printing, if the doctype is not the first node, it adds a newline. That has the same effect - gives a newline after these licensing comments.

Sorry for the late update!

@jhy jhy added the feature label Jun 19, 2022
@jhy jhy added this to the 1.15.2 milestone Jun 19, 2022
@hazendaz
Copy link
Contributor Author

hazendaz commented Jul 8, 2022

@jhy Finally got back to properly confirm, this worked as expected. This also seemed to fix another issue I had not noticed previously where I ended up with 2 license headers. Anyway, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants