Skip to content

test: complete goto_reply fixture and assertion (#11)#16

Merged
monkeyxite merged 9 commits into
mainfrom
fix/11-goto-reply-test
Jul 7, 2026
Merged

test: complete goto_reply fixture and assertion (#11)#16
monkeyxite merged 9 commits into
mainfrom
fix/11-goto-reply-test

Conversation

@monkeyxite

Copy link
Copy Markdown
Owner

Closes #11

Add tests/mail/fixtures/draft_with_quotes.txt with a realistic reply draft containing >-quoted lines, and rewrite the placeholder test to load it, call nav.goto_reply(), and assert the cursor lands on a line matching ^>.

All 82 tests pass.

Jonny Hou added 9 commits July 5, 2026 20:32
Was hardcoded to 0.12.2; glob-match any Cellar/neovim/*/share tree so
the tests keep running after nvim upgrades. Falls back to /tmp/plenary.nvim
for CI where the Cellar path does not exist.
- Extract collect_quoted_sig_ranges(lines) as a pure function that
  collects all '> --' ranges at any quoting depth (^>[ >]*%-%-).
  Testable without a real buffer.
- kill_quoted_sig iterates collected ranges bottom-to-top so earlier
  line indices remain valid after each nvim_buf_set_lines call.
- Broadened separator pattern from '^> ?%-%-' to '^>[ >]*%-%-' to
  handle doubly-quoted (and deeper) sig separators in long threads.
- Add two new tests: multi-block buffer test (fails old code, passes
  new) and a pure-function range-collection unit test.
- Add fixture draft_multi_quoted_sig.txt for reference.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors contact resolution, querying, and picking to run asynchronously using vim.system, extracting the resolver logic into a dedicated resolver.lua module. It also updates the calendar integration to use event-specific dates rather than the current date, improves signature stripping to handle multiple quoted signatures in long threads, and adds corresponding unit tests. The review feedback highlights two issues: a potential runtime error in contacts.lua if JSON decoding returns a non-table value, and a pattern matching bug in contacts_picker.lua that silently filters out contacts without a display name.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +84 to +85
local ok, data = pcall(vim.json.decode, output)
if not ok or not data then return {} end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If output is valid JSON but parses to a non-table value (such as a number, string, or boolean), data will not be a table. Calling ipairs(data) on line 87 will then throw a runtime error.

Using type(data) ~= 'table' instead of not data ensures robust handling of any unexpected JSON formats.

  local ok, data = pcall(vim.json.decode, output)
  if not ok or type(data) ~= 'table' then return {} end

local entries, seen = {}, {}
for _, line in ipairs(vim.split(stdout or '', '\n')) do
if line ~= '' then
local email, name = line:match('^([^\t]+)\t([^\t]+)')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The pattern ^([^\\t]+)\\t([^\\t]+) requires at least one non-tab character for the name field. If a contact has no display name (e.g., the line is email\\t or email\\t\\t), the match will fail entirely and return nil, causing these contacts to be silently omitted from the Telescope picker.

Changing the pattern to ^([^\\t]+)\\t([^\\t]*) allows matching empty name fields, returning "" for the name and correctly preserving the contact.

      local email, name = line:match('^([^\\t]+)\t([^\\t]*)')

@monkeyxite
monkeyxite merged commit 3e66d12 into main Jul 7, 2026
2 checks passed
@monkeyxite

Copy link
Copy Markdown
Owner Author

Superseded by #20 — the head SHA on this PR was stale (pointed at main baseline) due to a branch-pointer mishap during the parallel-agent rollout. GitHub auto-marked this PR merged. The actual #11 fix now lives on the same branch fix/11-goto-reply-test and is tracked in #20.

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.

test: navigate goto_reply test is a placeholder — needs a fixture with quoted lines

1 participant