Skip to content

🪲 BUG-#66: Group multi-clause operands of OR/NOT in IMAP query - #75

Merged
FernandoCelmer merged 2 commits into
developfrom
feature/66
May 17, 2026
Merged

🪲 BUG-#66: Group multi-clause operands of OR/NOT in IMAP query#75
FernandoCelmer merged 2 commits into
developfrom
feature/66

Conversation

@FernandoCelmer

Copy link
Copy Markdown
Member

Summary

  • Added _group() helper that wraps multi-clause IMAP search expressions in an outer parenthesis (single search-key group)
  • _Expr.__or__ and _Expr.__invert__ now group both operands so the resulting OR / NOT arity matches the Python-level expression
  • Single already-grouped clauses stay unwrapped to keep simple expressions readable
  • Same fix also resolves Query.exclude / Query.or_ / Query.__or__ / Query.__invert__ since they route through _Expr

Closes #66
Closes #72

Test plan

  • pytest tests/clients/imap/test_query.py — 16 passed

@FernandoCelmer FernandoCelmer left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🔍 Code Review

Code issues found: 0

_group plus _is_balanced correctly preserves IMAP OR/NOT arity. Worth noting the design choices for future readers:

  • Singletons stay unwrapped (OR (SUBJECT "a") (SUBJECT "b")) — readable, still RFC 3501 valid.
  • Multi-clause operands get one outer paren (OR ((SUBJECT "x") (UNSEEN)) (FROM "a@b")) — collapsed into a single search-key 'list' as the grammar requires.
  • _is_balanced toggles inside double quotes so a literal ) inside a value wouldn't break depth counting (not exploitable today since _ascii strips non-ASCII, but defensive).

Closes #66 and #72 in one move since Query.exclude/or_/__or__/__invert__ all route through _Expr.

@FernandoCelmer
FernandoCelmer merged commit 87e2185 into develop May 17, 2026
11 checks passed
@FernandoCelmer
FernandoCelmer deleted the feature/66 branch May 17, 2026 21:30
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