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

Added on_after_end_tag handler #109

Closed

Conversation

mitsuhiko
Copy link
Contributor

@mitsuhiko mitsuhiko commented Nov 29, 2021

This adds an on_after_end_tag handler to address #108. Note that when I made the commit, the pre-commit hook fired and reformatted the entire codebase as part 1.56.0 standard.

I'm not sure why the code base is currently not formatted to latest cargo fmt. As such this includes a lot of changes that are completely irrelevant to the actual change I was trying to make.

If this type of change is interesting I can address the code formatting situation and ad tests and docs.

@mitsuhiko mitsuhiko requested a review from a team as a code owner November 29, 2021 11:20
@mitsuhiko
Copy link
Contributor Author

So the challenge I'm now recognizing is that because a lot of HTML tags are optional there is no guarantee that you're going to have an end of tag callback. Take for instance a list of items (<li>). The closing li tag is optional.

@jyn514
Copy link
Contributor

jyn514 commented Dec 16, 2021

Thanks for the PR! #112 fixes the formatting, can you rebase over master so the PR only has your own changes?

This adds an on_after_end_tag handler to address #108.
So the challenge I'm now recognizing is that because a lot of HTML tags are optional there is no guarantee that you're going to have an end of tag callback. Take for instance a list of items (<li>). The closing li tag is optional.

Hmm, I think it makes sense to hold off on merging this until we figure out #110.

@mitsuhiko mitsuhiko force-pushed the feature/on-after-end-tag branch from ae820a1 to 829d384 Compare December 27, 2021 18:29
@mitsuhiko
Copy link
Contributor Author

I rebased it. I agree that merging this without addressing #110 is not immensely useful.

@mitsuhiko
Copy link
Contributor Author

This is heavily outdated so I will close this.

@mitsuhiko mitsuhiko closed this Dec 22, 2024
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.

2 participants