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

Optimizations #275

Merged
merged 3 commits into from
Jul 26, 2024
Merged

Optimizations #275

merged 3 commits into from
Jul 26, 2024

Conversation

aehlke
Copy link
Collaborator

@aehlke aehlke commented Jul 25, 2024

No description provided.

@scinfu
Copy link
Owner

scinfu commented Jul 26, 2024

thank you

@scinfu scinfu merged commit 91cc703 into scinfu:master Jul 26, 2024
0 of 4 checks passed
@aehlke
Copy link
Collaborator Author

aehlke commented Jul 27, 2024

thanks so much for reviewing / merging

the big one that needs work is consumeToAny. the unicode scalar vs utf8/utf16 indexing is half of it (and maybe the need to make strings utf8 contiguous, I didn't verify), the other is instantiating the new string. maybe it can be reworked to use substrings. I read through the source but couldn't confidently decide what to try

@@ -417,7 +422,7 @@ enum TokeniserState: TokeniserStateProtocol {
t.emit(TokeniserStateVars.replacementChar)
break
default:
let data = r.consumeToAny("-", UnicodeScalar.LessThan, TokeniserStateVars.nullScalr)
let data = r.consumeToAny(TokeniserStateVars.dataDefaultStopChars)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this and next change were my errors

aehlke added a commit to lake-of-fire/SwiftSoup that referenced this pull request Aug 2, 2024
aehlke added a commit that referenced this pull request Aug 15, 2024
* fix most tests

* utf-8 optimization

* fix regressions introduced by #275
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