Skip to content

docs: replace stale test-count badge with live workflow badge#15

Merged
monkeyxite merged 1 commit into
mainfrom
fix/12-readme-badge
Jul 7, 2026
Merged

docs: replace stale test-count badge with live workflow badge#15
monkeyxite merged 1 commit into
mainfrom
fix/12-readme-badge

Conversation

@monkeyxite

Copy link
Copy Markdown
Owner

Replaces the hardcoded tests-61%20passing shield badge with a live GitHub Actions workflow status badge.

Closes #12

@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 transitions contact querying and resolution to an asynchronous model, extracts contact resolution into a dedicated resolver module, refactors quoted signature removal to handle multiple blocks in long threads, and updates calendar, snippets, and spell-switching features. Comprehensive unit tests have been added to cover these async flows. The review feedback identifies several critical issues: a potential race condition during async buffer resolution, a bug in signature range collection that could cause data loss in nested threads, a potential runtime crash from nil concatenation, incomplete pattern escaping for domains with hyphens, and a redundant badge in the README.

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 +151 to +165
local function finish()
done_count = done_count + 1
if done_count < total then return end
for _, t in ipairs(tasks) do
if resolved_lines[t.idx] then lines[t.idx] = resolved_lines[t.idx] end
end
vim.schedule(function()
vim.api.nvim_buf_set_lines(bufnr, 0, header_end, false, vim.list_slice(lines, 1, header_end))
if #unresolved > 0 then
vim.notify('⚠ No email found for: ' .. table.concat(unresolved, ', '), vim.log.levels.WARN)
else
vim.notify('✓ All contacts resolved', vim.log.levels.INFO)
end
end)
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.

high

There is a potential race condition here. Since contact resolution is asynchronous (running external commands like khard, notmuch, and ldap), the user might edit the buffer or close it before the callbacks complete. Writing back the stale lines list will overwrite any concurrent user edits or fail if the buffer is invalid. To prevent this, check if the buffer is still valid and refetch the current lines to merge the resolved addresses safely.

  local function finish()
    done_count = done_count + 1
    if done_count < total then return end
    vim.schedule(function()
      if not vim.api.nvim_buf_is_valid(bufnr) then return end
      local current_lines = vim.api.nvim_buf_get_lines(bufnr, 0, -1, false)
      local current_header_end = 0
      for i, l in ipairs(current_lines) do
        if l == '' then current_header_end = i - 1; break end
      end
      for _, t in ipairs(tasks) do
        if t.idx <= current_header_end and resolved_lines[t.idx] then
          local cur_line = current_lines[t.idx]
          local prefix = t.line:match('^(%a+:)')
          if prefix and cur_line:match('^' .. prefix) then
            current_lines[t.idx] = resolved_lines[t.idx]
          end
        end
      end
      vim.api.nvim_buf_set_lines(bufnr, 0, current_header_end, false, vim.list_slice(current_lines, 1, current_header_end))
      if #unresolved > 0 then
        vim.notify('⚠ No email found for: ' .. table.concat(unresolved, ', '), vim.log.levels.WARN)
      else
        vim.notify('✓ All contacts resolved', vim.log.levels.INFO)
      end
    end)
  end

Comment on lines +71 to +84
function M.collect_quoted_sig_ranges(lines)
local ranges = {}
for i, l in ipairs(lines) do
if l:match('^> ?%-%-') then
start = i
if l:match('^>[ >]*%-%-') then
local stop = i
for j = i + 1, #lines do
if lines[j]:match('^>') then stop = j
else break end
end
ranges[#ranges + 1] = { start = i, stop = stop }
end
end
if start then
-- Find end of this quoted block
local stop = start
for i = start + 1, #lines do
if lines[i]:match('^>') then stop = i
else break end
end
vim.api.nvim_buf_set_lines(0, start - 1, stop, false, {})
return ranges
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.

high

The current implementation of collect_quoted_sig_ranges scans forward for any line starting with > to find the end of the signature. In a nested email thread, this causes a signature at a deeper quote level (e.g., > > --) to swallow subsequent lines at shallower quote levels (e.g., > Alice reply), leading to silent data loss when the signature is deleted. Restrict the scanning to lines that share the same quote prefix/depth.

function M.collect_quoted_sig_ranges(lines)
  local ranges = {}
  for i, l in ipairs(lines) do
    local prefix = l:match('^(>[ >]*)%-%-')
    if prefix then
      local stop = i
      for j = i + 1, #lines do
        if lines[j]:sub(1, #prefix) == prefix then
          stop = j
        else
          break
        end
      end
      ranges[#ranges + 1] = { start = i, stop = stop }
    end
  end
  return ranges
end

Comment thread README.md Outdated

[![Tests](https://github.com/monkeyxite/nvim-mail/actions/workflows/test.yml/badge.svg)](https://github.com/monkeyxite/nvim-mail/actions)
![Tests](https://img.shields.io/badge/tests-61%20passing-brightgreen?style=flat-square)
[![Tests](https://github.com/monkeyxite/nvim-mail/actions/workflows/test.yml/badge.svg)](https://github.com/monkeyxite/nvim-mail/actions/workflows/test.yml)

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 live workflow badge on line 6 is redundant because an identical live workflow badge already exists on line 5. Consider removing this duplicate badge.

local norm = r.normalize_name(name)
local parts = vim.split(norm, ' ', { trimempty = true })
local work_domain = require('nvim-mail.contacts').config.work_domain or 'example.com'
local domain_pat = work_domain:gsub('%.', '%%.')

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

Using work_domain:gsub('%.', '%%.') only escapes dots. If the domain contains other magic characters (such as hyphens -), the pattern matching in notmuch_lookup may fail or behave unexpectedly. Use a more robust pattern escape that handles all non-alphanumeric characters.

  local domain_pat = work_domain:gsub('([^%w])', '%%%1')

local function done(email)
if found then return end
found = true
if email then save_to_khard(parts[1], parts[#parts], email) 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 parts is empty (e.g., if name is empty or cannot be normalized), parts[1] and parts[#parts] will be nil. This will cause a runtime error in save_to_khard due to nil concatenation. Add safe fallbacks similar to those used on line 90.

    if email then save_to_khard(parts[1] or name, parts[#parts] or '', email) end

@monkeyxite
monkeyxite force-pushed the fix/12-readme-badge branch from f9a0e35 to ade6cfe Compare July 7, 2026 14:19
@monkeyxite

Copy link
Copy Markdown
Owner Author

Addressed. Amended the commit to remove the duplicate Tests workflow badge; the pre-existing badge on line 5 is kept and the stale hardcoded one is deleted. Features paragraph tweak retained.

Note: Gemini Code Assist consumer version retires 2026-07-17.

@monkeyxite
monkeyxite merged commit bbc4500 into main Jul 7, 2026
2 checks passed
@monkeyxite
monkeyxite deleted the fix/12-readme-badge branch July 7, 2026 14:24
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.

docs: README test badge count is hardcoded and stale

1 participant