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

REFACTOR: Using html parser to update index.html file #396

Merged
merged 6 commits into from Aug 14, 2022
Merged

REFACTOR: Using html parser to update index.html file #396

merged 6 commits into from Aug 14, 2022

Conversation

OutdatedGuy
Copy link
Contributor

Changes

  • Using html package to read index.html file as a document tree
  • Modifying content using document methods
  • Writing document back to index.html file

Advantages

  • Easy to understand code
  • Would still work if index.html file is edited by user
  • Cleaner code

@jonbhanson
Copy link
Owner

@OutdatedGuy I really like the work you did here, thanks! I agree that your approach is a better one. However, the thing that scares me is that html hasn't been updated in a year and a half, and it has a lot of outstanding issues. I wonder if it is preferable to keep the bad code I know instead of the potential worse problems with html that I don't know?

@OutdatedGuy
Copy link
Contributor Author

I got your point. The html package is indeed not maintained.

So how about publishing this commit as a breaking change pre-release version. Like 3.0.0-beta
So others can manually test it.

@OutdatedGuy
Copy link
Contributor Author

@jonbhanson any updates on this?

I manually checked the package for different html formats and it seems to work fine. Also as this is a flutter project, generally the index.html file won't have much included, so I don't see any problem for now.

@jonbhanson jonbhanson merged commit 68da2c4 into jonbhanson:master Aug 14, 2022
jonbhanson added a commit that referenced this pull request Aug 21, 2022
…eadme. Thanks Muhammad for PR #407. Use html parser. Thanks OutdatedGuy for PR #396. Separate branding property for Android 12. Closes #405.
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.

None yet

2 participants