Skip to content

♻️ Backport ResponseReaderto v0.3 #439

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

Merged
merged 11 commits into from
Apr 21, 2025

Conversation

nevans added 11 commits April 20, 2025 18:20
This also allows us to check against the concatenated buffer, rather
than the smaller line buffer.  That distinction doesn't really matter
now, since we always read an entire line at once.  But it will matter if
we read partial lines.
IMO, this refactoring makes `get_response` much easier to understand.
Which will be useful, because I'm about to complicate it.  😉
We know exactly how much memory we're going to need, so we can allocate
this up-front, and save a few malloc/memcpy calls on larger literals.
This feels a lot more self-documenting than returning nil then breaking
when nil is returned.  Also, it lets me refactor the return values for
the get_response_line/get_response_literal methods, or throw from even
deeper in the stack.
It's nice to extract a little bit of the complexity from the core
`Net::IMAP` class.  But my primary motivation was so that I could
directly test this code quickly and in isolation from needing to
simulate a full IMAP connection.
`Net::IMAP::Config` was introduced by `net-imap` v0.4.
This avoids the need to pass these to every method that uses them.
That's not a big deal now, but it simplifies the next few changes.

Also added a missing test for empty literals: "{0}\r\n"
@nevans nevans merged commit 4d1206e into v0.3-stable Apr 21, 2025
34 checks passed
@nevans nevans deleted the backport/v0.3-response_reader branch April 21, 2025 00:47
@nevans nevans added the backport This issue or PR is for a stable release branch label Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport This issue or PR is for a stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant