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

Support b and i tags in Basic HTML #4

Closed
wants to merge 2 commits into from
Closed

Support b and i tags in Basic HTML #4

wants to merge 2 commits into from

Conversation

ryanbooker
Copy link

@ryanbooker ryanbooker commented Apr 26, 2024

The <b> and <i> tags are still quite common. This PR adds support for them to the BasicHTML parser.

Copy link
Owner

Choose a reason for hiding this comment

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

Why has the package platform been bumped to 16? It should continue supporting the lowest possible version.

Copy link
Author

@ryanbooker ryanbooker Apr 26, 2024

Choose a reason for hiding this comment

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

Because it was failing to compile on iOS. macOS was compiling fine.

The Regex at BasicHTML.swift:96 & 98 seems to be the culprit.

../BasicHTML.swift:96:37: 'Regex' is only available in iOS 16.0 or newer
../BasicHTML.swift:96:54: 'firstMatch(in:)' is only available in iOS 16.0 or newer
../BasicHTML.swift:98:42: 'output' is only available in iOS 16.0 or newer

I think it would have been passing the checks because the build is done on macOS 13.

Copy link
Owner

Choose a reason for hiding this comment

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

That makes sense, I'll push another merge after this to fix the platforms for macOS. As for this or it looks good!

Copy link
Author

@ryanbooker ryanbooker Apr 27, 2024

Choose a reason for hiding this comment

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

Now worries. macOS seems ok as Regex exists in macOS 13.

Is there anything you want me to change?

@ActuallyTaylor
Copy link
Owner

Thank you for the PR! I just have on comment about the package version, other than that it looks great :)

@ryanbooker ryanbooker closed this by deleting the head repository Jun 6, 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