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: cleaner header parsing #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AaronO
Copy link
Contributor

@AaronO AaronO commented Apr 24, 2023

Builds off #134 (swar), #138 (Bytes cursor)

Cleaner, faster and less macros !

Key changes

  • Broke down header-parsing into clean conceptual steps (whilst being faster !)
  • Added InnerResult allowing for idiomatic ? early-exits, removing need for parsing-helper macros
  • Removed macros.rs, leaving only byte_map! (response header-parsing macros should become functions)

TODO

  • convert response header-parser, supporting its quirks

@AaronO AaronO force-pushed the refactor/header-parsing branch 3 times, most recently from c9d4c3a to 9c27ddc Compare April 25, 2023 19:07
@seanmonstar
Copy link
Owner

Happy to see those gnarly macros cleaned up. I put those together when I was very new at Rust, and slowly grew them poorly, and understanding them whenever I came back to them was always confusing.

I'm guessing moving around the enums makes them slightly smaller. You said there was a measurable perf improvement?

Builds off seanmonstar#134 (swar), seanmonstar#138 (Bytes cursor)

Cleaner, faster and less macros !

- Broke down header-parsing into clean conceptual steps (whilst being faster !)
- Added InnerResult allowing for idiomatic `?` early-exits, removing need for parsing-helper macros
- Removed macros.rs, leaving only `byte_map!` (response header-parsing macros should become functions)
@seanmonstar
Copy link
Owner

Just checking in here, did you want to get this merged before we shoot out the new release?

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