feat(forge): add validate option for forge ref validation on write (#138)
Problem: Typos in forge refs like `gh:user/repo#42` silently persist — there's no feedback when a ref points to a nonexistent issue. Solution: Add `forge.validate` config option. When enabled, `diff.apply()` returns new/changed `ForgeRef[]` and `forge.validate_refs()` fetches metadata for each, logging specific warnings for not-found, auth, or CLI-missing errors.
This commit is contained in:
parent
494e26d7a1
commit
a4f782a5fb
6 changed files with 163 additions and 19 deletions
|
|
@ -1501,6 +1501,7 @@ Configuration: ~
|
||||||
vim.g.pending = {
|
vim.g.pending = {
|
||||||
forge = {
|
forge = {
|
||||||
close = false,
|
close = false,
|
||||||
|
validate = false,
|
||||||
warn_missing_cli = true,
|
warn_missing_cli = true,
|
||||||
github = {
|
github = {
|
||||||
icon = '',
|
icon = '',
|
||||||
|
|
@ -1525,6 +1526,10 @@ Top-level fields: ~
|
||||||
{close} (boolean, default: false) When true, tasks linked to
|
{close} (boolean, default: false) When true, tasks linked to
|
||||||
closed/merged remote issues are automatically marked
|
closed/merged remote issues are automatically marked
|
||||||
done on buffer open.
|
done on buffer open.
|
||||||
|
{validate} (boolean, default: false) When true, new or changed
|
||||||
|
forge refs are validated on `:w` by fetching metadata.
|
||||||
|
Logs a warning if the ref is not found, auth fails, or
|
||||||
|
the CLI is missing.
|
||||||
{warn_missing_cli} (boolean, default: true) When true, warns once per
|
{warn_missing_cli} (boolean, default: true) When true, warns once per
|
||||||
forge per session if the CLI is missing or fails.
|
forge per session if the CLI is missing or fails.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -41,6 +41,7 @@
|
||||||
|
|
||||||
---@class pending.ForgeConfig
|
---@class pending.ForgeConfig
|
||||||
---@field close? boolean
|
---@field close? boolean
|
||||||
|
---@field validate? boolean
|
||||||
---@field warn_missing_cli? boolean
|
---@field warn_missing_cli? boolean
|
||||||
---@field [string] pending.ForgeInstanceConfig
|
---@field [string] pending.ForgeInstanceConfig
|
||||||
|
|
||||||
|
|
@ -155,6 +156,7 @@ local defaults = {
|
||||||
sync = {},
|
sync = {},
|
||||||
forge = {
|
forge = {
|
||||||
close = false,
|
close = false,
|
||||||
|
validate = false,
|
||||||
warn_missing_cli = true,
|
warn_missing_cli = true,
|
||||||
github = {
|
github = {
|
||||||
icon = '',
|
icon = '',
|
||||||
|
|
|
||||||
|
|
@ -82,14 +82,26 @@ function M.parse_buffer(lines)
|
||||||
return result
|
return result
|
||||||
end
|
end
|
||||||
|
|
||||||
|
---@param a? pending.ForgeRef
|
||||||
|
---@param b? pending.ForgeRef
|
||||||
|
---@return boolean
|
||||||
|
local function refs_equal(a, b)
|
||||||
|
if not a or not b then
|
||||||
|
return false
|
||||||
|
end
|
||||||
|
return a.forge == b.forge and a.owner == b.owner
|
||||||
|
and a.repo == b.repo and a.number == b.number
|
||||||
|
end
|
||||||
|
|
||||||
---@param lines string[]
|
---@param lines string[]
|
||||||
---@param s pending.Store
|
---@param s pending.Store
|
||||||
---@param hidden_ids? table<integer, true>
|
---@param hidden_ids? table<integer, true>
|
||||||
---@return nil
|
---@return pending.ForgeRef[]
|
||||||
function M.apply(lines, s, hidden_ids)
|
function M.apply(lines, s, hidden_ids)
|
||||||
local parsed = M.parse_buffer(lines)
|
local parsed = M.parse_buffer(lines)
|
||||||
local now = timestamp()
|
local now = timestamp()
|
||||||
local data = s:data()
|
local data = s:data()
|
||||||
|
local new_refs = {} ---@type pending.ForgeRef[]
|
||||||
|
|
||||||
local old_by_id = {}
|
local old_by_id = {}
|
||||||
for _, task in ipairs(data.tasks) do
|
for _, task in ipairs(data.tasks) do
|
||||||
|
|
@ -120,6 +132,9 @@ function M.apply(lines, s, hidden_ids)
|
||||||
order = order_counter,
|
order = order_counter,
|
||||||
_extra = entry.forge_ref and { _forge_ref = entry.forge_ref } or nil,
|
_extra = entry.forge_ref and { _forge_ref = entry.forge_ref } or nil,
|
||||||
})
|
})
|
||||||
|
if entry.forge_ref then
|
||||||
|
table.insert(new_refs, entry.forge_ref)
|
||||||
|
end
|
||||||
else
|
else
|
||||||
seen_ids[entry.id] = true
|
seen_ids[entry.id] = true
|
||||||
local task = old_by_id[entry.id]
|
local task = old_by_id[entry.id]
|
||||||
|
|
@ -154,6 +169,10 @@ function M.apply(lines, s, hidden_ids)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
if entry.forge_ref ~= nil then
|
if entry.forge_ref ~= nil then
|
||||||
|
local old_ref = task._extra and task._extra._forge_ref or nil
|
||||||
|
if not refs_equal(old_ref, entry.forge_ref) then
|
||||||
|
table.insert(new_refs, entry.forge_ref)
|
||||||
|
end
|
||||||
if not task._extra then
|
if not task._extra then
|
||||||
task._extra = {}
|
task._extra = {}
|
||||||
end
|
end
|
||||||
|
|
@ -188,6 +207,9 @@ function M.apply(lines, s, hidden_ids)
|
||||||
order = order_counter,
|
order = order_counter,
|
||||||
_extra = entry.forge_ref and { _forge_ref = entry.forge_ref } or nil,
|
_extra = entry.forge_ref and { _forge_ref = entry.forge_ref } or nil,
|
||||||
})
|
})
|
||||||
|
if entry.forge_ref then
|
||||||
|
table.insert(new_refs, entry.forge_ref)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
::continue::
|
::continue::
|
||||||
|
|
@ -202,6 +224,7 @@ function M.apply(lines, s, hidden_ids)
|
||||||
end
|
end
|
||||||
|
|
||||||
s:save()
|
s:save()
|
||||||
|
return new_refs
|
||||||
end
|
end
|
||||||
|
|
||||||
return M
|
return M
|
||||||
|
|
|
||||||
|
|
@ -243,30 +243,51 @@ function M.format_label(ref, cache)
|
||||||
return text, hl
|
return text, hl
|
||||||
end
|
end
|
||||||
|
|
||||||
|
---@class pending.ForgeFetchError
|
||||||
|
---@field code 'not_found'|'auth'|'cli_missing'|'network'
|
||||||
|
---@field message string
|
||||||
|
|
||||||
---@param ref pending.ForgeRef
|
---@param ref pending.ForgeRef
|
||||||
---@param callback fun(cache: pending.ForgeCache?)
|
---@param callback fun(cache: pending.ForgeCache?, err: pending.ForgeFetchError?)
|
||||||
function M.fetch_metadata(ref, callback)
|
function M.fetch_metadata(ref, callback)
|
||||||
local args = M._api_args(ref)
|
local args = M._api_args(ref)
|
||||||
|
local backend = _by_name[ref.forge]
|
||||||
|
|
||||||
|
if backend and vim.fn.executable(backend.cli) == 0 then
|
||||||
|
vim.schedule(function()
|
||||||
|
local forge_cfg = config.get().forge or {}
|
||||||
|
if forge_cfg.warn_missing_cli ~= false and not backend._warned then
|
||||||
|
backend._warned = true
|
||||||
|
log.warn(('%s not installed'):format(backend.cli))
|
||||||
|
end
|
||||||
|
callback(nil, { code = 'cli_missing', message = backend.cli .. ' not installed' })
|
||||||
|
end)
|
||||||
|
return
|
||||||
|
end
|
||||||
|
|
||||||
vim.system(args, { text = true }, function(result)
|
vim.system(args, { text = true }, function(result)
|
||||||
if result.code ~= 0 or not result.stdout or result.stdout == '' then
|
if result.code ~= 0 or not result.stdout or result.stdout == '' then
|
||||||
local forge_cfg = config.get().forge or {}
|
local stderr = result.stderr or ''
|
||||||
local backend = _by_name[ref.forge]
|
local err_code = 'network' ---@type 'not_found'|'auth'|'network'
|
||||||
if backend and forge_cfg.warn_missing_cli ~= false and not backend._warned then
|
if stderr:find('404') or stderr:find('Not Found') then
|
||||||
backend._warned = true
|
err_code = 'not_found'
|
||||||
vim.system(backend.auth_status_args, { text = true }, function(auth_result)
|
elseif stderr:find('401') or stderr:find('requires authentication') then
|
||||||
vim.schedule(function()
|
err_code = 'auth'
|
||||||
if auth_result.code ~= 0 then
|
|
||||||
log.warn(('%s not authenticated — run `%s`'):format(backend.cli, backend.auth_cmd))
|
|
||||||
end
|
|
||||||
callback(nil)
|
|
||||||
end)
|
|
||||||
end)
|
|
||||||
else
|
|
||||||
vim.schedule(function()
|
|
||||||
callback(nil)
|
|
||||||
end)
|
|
||||||
end
|
end
|
||||||
|
vim.schedule(function()
|
||||||
|
local forge_cfg = config.get().forge or {}
|
||||||
|
if
|
||||||
|
backend
|
||||||
|
and forge_cfg.warn_missing_cli ~= false
|
||||||
|
and not backend._warned
|
||||||
|
then
|
||||||
|
backend._warned = true
|
||||||
|
log.warn(
|
||||||
|
('%s not found or not authenticated — run `%s`'):format(backend.cli, backend.auth_cmd)
|
||||||
|
)
|
||||||
|
end
|
||||||
|
callback(nil, { code = err_code, message = stderr })
|
||||||
|
end)
|
||||||
return
|
return
|
||||||
end
|
end
|
||||||
local ok, decoded = pcall(vim.json.decode, result.stdout)
|
local ok, decoded = pcall(vim.json.decode, result.stdout)
|
||||||
|
|
@ -567,4 +588,31 @@ M.register({
|
||||||
end,
|
end,
|
||||||
})
|
})
|
||||||
|
|
||||||
|
---@param refs pending.ForgeRef[]
|
||||||
|
function M.validate_refs(refs)
|
||||||
|
local forge_cfg = config.get().forge or {}
|
||||||
|
if not forge_cfg.validate then
|
||||||
|
return
|
||||||
|
end
|
||||||
|
for _, ref in ipairs(refs) do
|
||||||
|
M.fetch_metadata(ref, function(_, err)
|
||||||
|
if err then
|
||||||
|
local label = ref.owner .. '/' .. ref.repo .. '#' .. ref.number
|
||||||
|
local backend = _by_name[ref.forge]
|
||||||
|
if err.code == 'not_found' then
|
||||||
|
log.warn(('%s not found — check owner, repo, and number'):format(label))
|
||||||
|
elseif err.code == 'auth' then
|
||||||
|
local cmd = backend and backend.auth_cmd or 'auth'
|
||||||
|
log.warn(('%s: not authenticated — run `%s`'):format(label, cmd))
|
||||||
|
elseif err.code == 'cli_missing' then
|
||||||
|
local cli = backend and backend.cli or 'cli'
|
||||||
|
log.warn(('%s: %s not installed'):format(label, cli))
|
||||||
|
else
|
||||||
|
log.warn(('%s: validation failed'):format(label))
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
return M
|
return M
|
||||||
|
|
|
||||||
|
|
@ -489,9 +489,13 @@ function M._on_write(bufnr)
|
||||||
if #stack > UNDO_MAX then
|
if #stack > UNDO_MAX then
|
||||||
table.remove(stack, 1)
|
table.remove(stack, 1)
|
||||||
end
|
end
|
||||||
diff.apply(lines, s, hidden)
|
local new_refs = diff.apply(lines, s, hidden)
|
||||||
M._recompute_counts()
|
M._recompute_counts()
|
||||||
buffer.render(bufnr)
|
buffer.render(bufnr)
|
||||||
|
if new_refs and #new_refs > 0 then
|
||||||
|
local forge = require('pending.forge')
|
||||||
|
forge.validate_refs(new_refs)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
---@return nil
|
---@return nil
|
||||||
|
|
|
||||||
|
|
@ -275,6 +275,68 @@ describe('diff', function()
|
||||||
assert.are.equal('completion', tasks[1].recur_mode)
|
assert.are.equal('completion', tasks[1].recur_mode)
|
||||||
end)
|
end)
|
||||||
|
|
||||||
|
it('returns forge refs for new tasks', function()
|
||||||
|
local lines = {
|
||||||
|
'# Inbox',
|
||||||
|
'- [ ] Fix bug gh:user/repo#42',
|
||||||
|
}
|
||||||
|
local refs = diff.apply(lines, s)
|
||||||
|
assert.are.equal(1, #refs)
|
||||||
|
assert.are.equal('github', refs[1].forge)
|
||||||
|
assert.are.equal(42, refs[1].number)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('returns forge refs for changed refs on existing tasks', function()
|
||||||
|
s:add({ description = 'Fix bug gh:user/repo#1', _extra = {
|
||||||
|
_forge_ref = { forge = 'github', owner = 'user', repo = 'repo', type = 'issue', number = 1, url = '' },
|
||||||
|
} })
|
||||||
|
s:save()
|
||||||
|
local lines = {
|
||||||
|
'# Todo',
|
||||||
|
'/1/- [ ] Fix bug gh:user/repo#99',
|
||||||
|
}
|
||||||
|
local refs = diff.apply(lines, s)
|
||||||
|
assert.are.equal(1, #refs)
|
||||||
|
assert.are.equal(99, refs[1].number)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('returns empty when forge ref is unchanged', function()
|
||||||
|
s:add({ description = 'Fix bug gh:user/repo#42', _extra = {
|
||||||
|
_forge_ref = { forge = 'github', owner = 'user', repo = 'repo', type = 'issue', number = 42, url = '' },
|
||||||
|
} })
|
||||||
|
s:save()
|
||||||
|
local lines = {
|
||||||
|
'# Todo',
|
||||||
|
'/1/- [ ] Fix bug gh:user/repo#42',
|
||||||
|
}
|
||||||
|
local refs = diff.apply(lines, s)
|
||||||
|
assert.are.equal(0, #refs)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('returns empty for tasks without forge refs', function()
|
||||||
|
local lines = {
|
||||||
|
'# Inbox',
|
||||||
|
'- [ ] Plain task',
|
||||||
|
}
|
||||||
|
local refs = diff.apply(lines, s)
|
||||||
|
assert.are.equal(0, #refs)
|
||||||
|
end)
|
||||||
|
|
||||||
|
it('returns forge refs for duplicated tasks', function()
|
||||||
|
s:add({ description = 'Fix bug gh:user/repo#42', _extra = {
|
||||||
|
_forge_ref = { forge = 'github', owner = 'user', repo = 'repo', type = 'issue', number = 42, url = '' },
|
||||||
|
} })
|
||||||
|
s:save()
|
||||||
|
local lines = {
|
||||||
|
'# Todo',
|
||||||
|
'/1/- [ ] Fix bug gh:user/repo#42',
|
||||||
|
'/1/- [ ] Fix bug gh:user/repo#42',
|
||||||
|
}
|
||||||
|
local refs = diff.apply(lines, s)
|
||||||
|
assert.are.equal(1, #refs)
|
||||||
|
assert.are.equal(42, refs[1].number)
|
||||||
|
end)
|
||||||
|
|
||||||
it('clears priority when [N] is removed from buffer line', function()
|
it('clears priority when [N] is removed from buffer line', function()
|
||||||
s:add({ description = 'Task name', priority = 1 })
|
s:add({ description = 'Task name', priority = 1 })
|
||||||
s:save()
|
s:save()
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue