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

HTML Processing Fixes #38

Merged
merged 2 commits into from
Apr 28, 2024
Merged

HTML Processing Fixes #38

merged 2 commits into from
Apr 28, 2024

Conversation

newsch
Copy link
Collaborator

@newsch newsch commented Jan 24, 2024

  • Add tests for each case

Copy link
Member

@biodranik biodranik left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@biodranik
Copy link
Member

@newsch should these changes be merged?

@newsch
Copy link
Collaborator Author

newsch commented Apr 21, 2024

Not yet, I'm trying to find some sample articles for tests.

@biodranik
Copy link
Member

Should it be rebased?

newsch added 2 commits April 27, 2024 21:08
Signed-off-by: Evan Lloyd New-Schmidt <[email protected]>
Not all languages have a nice `section > h2` structure.

Signed-off-by: Evan Lloyd New-Schmidt <[email protected]>
@newsch newsch force-pushed the processing-fixes branch from b0d057b to ad7052d Compare April 28, 2024 01:09
@newsch newsch marked this pull request as ready for review April 28, 2024 01:09
@newsch
Copy link
Collaborator Author

newsch commented Apr 28, 2024

From my tests the wrapper div handling was roughly a wash. Some data-heavy articles use them for custom lists and tables which greatly increase the size for little value. I've removed it for now until I can find better heuristics for it.

@newsch newsch merged commit cb835fc into main Apr 28, 2024
1 check passed
@newsch newsch deleted the processing-fixes branch April 28, 2024 01:12
@biodranik
Copy link
Member

Thanks! It's better to avoid storing large chunks of data, most people won't read them anyway from a mobile device on the go.

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