Skip to content

♻️ Extract handle_response from receive_responses#695

Merged
nevans merged 1 commit into
masterfrom
refactor-extract-handle_response-from-receive_responses
Jun 8, 2026
Merged

♻️ Extract handle_response from receive_responses#695
nevans merged 1 commit into
masterfrom
refactor-extract-handle_response-from-receive_responses

Conversation

@nevans

@nevans nevans commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Extracting handle_response makes the receive_responses much easier to read, IMO. Also, this block of code has been surrounded by a redundant begin/end, since #694 pulled the rescue clause inside the synchronize block.

connection_closed was only assigned by this portion of the loop, so it's been converted into the inverse of the return value for handle_response, and the loop is now a simple Kernel#loop. The receive_responses method returns when handle_response returns falsey.

`connection_closed` was only assigned by this portion of the loop, so
it's been converted into the return value, and the loop is now a simple
`Kernel#loop`.  The `receive_responses` method returns when
`handle_response` returns falsey.
Comment thread lib/net/imap.rb
@exception = nil unless connection_closed
end
end
handle_response(resp) or return

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It would make sense to convert this boolean loop breaker into a throw or raise. But the caveat is that we still need to reset @exception after handling an exception that doesn't closing the connection. So we'd still need to track that inside handle_response.

@nevans nevans changed the title ♻️ Extract handle_response from receive_responses ♻️ Extract handle_response from receive_responses Jun 8, 2026
@nevans nevans merged commit 33d8592 into master Jun 8, 2026
39 checks passed
@nevans nevans deleted the refactor-extract-handle_response-from-receive_responses branch June 8, 2026 14:55
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.

1 participant