Skip to content

fix: improve async format module #230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
glepnir wants to merge 1 commit into from
Closed

fix: improve async format module #230

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
274 changes: 145 additions & 129 deletions lua/guard/format.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
local async = require('guard._async')
local util = require('guard.util')
local filetype = require('guard.filetype')
local api = vim.api

local M = {}
local api, iter, filter = vim.api, vim.iter, vim.tbl_filter

local function save_views(bufnr)
local views = {}
Expand All @@ -21,107 +19,42 @@ local function restore_views(views)
end
end

local function update_buffer(bufnr, prev_lines, new_lines, srow, erow, old_indent)
if not new_lines or #new_lines == 0 then
local function update_buffer(bufnr, prev_content, new_content, srow, erow, old_indent)
if not new_content or #new_content == 0 then
return
end

local views = save_views(bufnr)
new_lines = vim.split(new_lines, '\r?\n')
if new_lines[#new_lines] == '' then
new_lines[#new_lines] = nil
end
-- Always update if content changed (compare strings directly)
if prev_content ~= new_content then
local views = save_views(bufnr)

local new_lines = vim.split(new_content, '\r?\n')
if new_lines[#new_lines] == '' then
new_lines[#new_lines] = nil
end

if not vim.deep_equal(new_lines, prev_lines) then
api.nvim_buf_set_lines(bufnr, srow, erow, false, new_lines)

if util.getopt('save_on_fmt') then
api.nvim_command('silent! noautocmd write!')
end

if old_indent then
vim.cmd(('silent %d,%dleft'):format(srow + 1, erow))
end

restore_views(views)
end
end

local function emit_event(status, data)
util.doau('GuardFmt', vim.tbl_extend('force', { status = status }, data or {}))
end

local function fail(msg)
emit_event('failed', { msg = msg })
util.doau('GuardFmt', {
status = 'failed',
msg = msg,
})
vim.notify('[Guard]: ' .. msg, vim.log.levels.WARN)
end

---Apply a single pure formatter
---@async
---@param buf number
---@param range table?
---@param config table
---@param fname string
---@param cwd string
---@param input string
---@return string? output
---@return string? error_msg
local function apply_pure_formatter(buf, range, config, fname, cwd, input)
-- Eval dynamic args
local cfg = vim.tbl_extend('force', {}, config)
if type(cfg.args) == 'function' then
cfg.args = cfg.args(buf)
end

if cfg.fn then
return cfg.fn(buf, range, input), nil
end

local result = async.await(1, function(callback)
local handle = vim.system(util.get_cmd(cfg, fname, buf), {
stdin = true,
cwd = cwd,
env = cfg.env,
timeout = cfg.timeout,
}, callback)
handle:write(input)
handle:write(nil)
end)

if result.code ~= 0 and #result.stderr > 0 then
return nil, ('%s exited with code %d\n%s'):format(cfg.cmd, result.code, result.stderr)
end

return result.stdout, nil
end

---Apply a single impure formatter
---@async
---@param buf number
---@param config table
---@param fname string
---@param cwd string
---@return string? error_msg
local function apply_impure_formatter(buf, config, fname, cwd)
-- Eval dynamic args
local cfg = vim.tbl_extend('force', {}, config)
if type(cfg.args) == 'function' then
cfg.args = cfg.args(buf)
end

local result = async.await(1, function(callback)
vim.system(util.get_cmd(cfg, fname, buf), {
text = true,
cwd = cwd,
env = cfg.env or {},
timeout = cfg.timeout,
}, callback)
end)

if result.code ~= 0 and #result.stderr > 0 then
return ('%s exited with code %d\n%s'):format(cfg.cmd, result.code, result.stderr)
end

return nil
end

local function do_fmt(buf)
buf = buf or api.nvim_get_current_buf()
local ft_conf = filetype[vim.bo[buf].filetype]
Expand All @@ -131,7 +64,6 @@ local function do_fmt(buf)
return
end

-- Get format range
local srow, erow = 0, -1
local range = nil
local mode = api.nvim_get_mode().mode
Expand All @@ -141,70 +73,116 @@ local function do_fmt(buf)
erow = range['end'][1]
end

local old_indent = (mode == 'V') and vim.fn.indent(srow + 1) or nil
local old_indent
if mode == 'V' then
old_indent = vim.fn.indent(srow + 1)
end

-- Get and filter configs
local fmt_configs = util.eval(ft_conf.formatter)
local fname, cwd = util.buf_get_info(buf)

fmt_configs = vim.tbl_filter(function(config)
fmt_configs = filter(function(config)
return util.should_run(config, buf)
end, fmt_configs)

-- Check executability
for _, config in ipairs(fmt_configs) do
local all_executable = not iter(fmt_configs):any(function(config)
if config.cmd and vim.fn.executable(config.cmd) ~= 1 then
util.report_error(config.cmd .. ' not executable')
return
return true
end
return false
end)

if not all_executable then
return
end

-- Classify formatters
local pure = vim.tbl_filter(function(config)
local pure = filter(function(config)
return config.fn or (config.cmd and config.stdin)
end, fmt_configs)

local impure = vim.tbl_filter(function(config)
local impure = filter(function(config)
return config.cmd and not config.stdin
end, fmt_configs)

-- Check range formatting compatibility
if range and #impure > 0 then
local impure_cmds = vim.tbl_map(function(c)
return c.cmd
end, impure)
util.report_error('Cannot apply range formatting for filetype ' .. vim.bo[buf].filetype)
local impure_cmds = {}
for _, config in ipairs(impure) do
table.insert(impure_cmds, config.cmd)
end
util.report_error(table.concat(impure_cmds, ', ') .. ' does not support reading from stdin')
return
end

emit_event('pending', { using = fmt_configs })
util.doau('GuardFmt', {
status = 'pending',
using = fmt_configs,
})

local prev_lines = api.nvim_buf_get_lines(buf, srow, erow, false)
local prev_content = table.concat(prev_lines, '\n')
Comment on lines +123 to +124
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

There's a potential issue with capturing prev_lines and prev_content at lines 123-124. These are captured outside the async.run block, before the changedtick is initialized inside async.run at line 130-134. If the buffer changes between line 124 (capturing content) and line 134 (capturing changedtick), the changedtick validation at line 189 would pass but prev_content would be stale. Consider moving lines 123-124 inside the async.run block, within the same scheduled callback as the changedtick capture (lines 130-134) to ensure atomic capture of both values.

Copilot uses AI. Check for mistakes.
local new_lines = prev_content
local errno = nil

async.run(function()
-- Initialize changedtick BEFORE any formatting (explicitly wait)
-- Explicitly wait for changedtick initialization
local changedtick = async.await(1, function(callback)
vim.schedule(function()
callback(api.nvim_buf_get_changedtick(buf))
end)
end)

local prev_lines = api.nvim_buf_get_lines(buf, srow, erow, false)
local new_lines = table.concat(prev_lines, '\n')

-- Apply pure formatters sequentially
for _, config in ipairs(pure) do
local output, err = apply_pure_formatter(buf, range, config, fname, cwd, new_lines)
if err then
fail(err)
return
if errno then
break
end

-- Eval dynamic args
local cfg = vim.tbl_extend('force', {}, config)
if type(cfg.args) == 'function' then
cfg.args = cfg.args(buf)
end

if cfg.fn then
new_lines = cfg.fn(buf, range, new_lines)
else
local result = async.await(1, function(callback)
local handle = vim.system(util.get_cmd(cfg, fname, buf), {
stdin = true,
cwd = cwd,
env = cfg.env,
timeout = cfg.timeout,
}, callback)
handle:write(new_lines)
handle:write(nil)
end)

if result.code ~= 0 and #result.stderr > 0 then
errno = {
cmd = cfg.cmd,
code = result.code,
stderr = result.stderr,
reason = cfg.cmd .. ' exited with errors',
}
else
new_lines = result.stdout
end
end
new_lines = output
end

-- Wait for schedule and update buffer
async.await(1, function(callback)
vim.schedule(function()
if not api.nvim_buf_is_valid(buf) then
fail('buffer no longer valid')
if errno then
if errno.reason:match('exited with errors$') then
fail(('%s exited with code %d\n%s'):format(errno.cmd, errno.code, errno.stderr))
Comment on lines +179 to +180
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

The error checking at line 179 uses pattern matching with match('exited with errors$') to distinguish error types, but this is fragile. The error reason is set as a string literal at line 167 ('exited with errors'), so the pattern matching is unnecessary - a direct string comparison would be more reliable and maintainable. Alternatively, use a separate error type field instead of parsing the reason string.

Copilot uses AI. Check for mistakes.
elseif errno.reason == 'buffer changed' then
fail('buffer changed during formatting')
else
fail(errno.reason)
end
callback()
return
end
Expand All @@ -215,42 +193,80 @@ local function do_fmt(buf)
return
end

update_buffer(buf, prev_lines, new_lines, srow, erow, old_indent)
if not api.nvim_buf_is_valid(buf) then
fail('buffer no longer valid')
callback()
return
end

update_buffer(buf, prev_content, new_lines, srow, erow, old_indent)
callback()
end)
end)

-- Stop if there was an error
if errno then
return
end
Comment on lines +208 to +210
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

The error handling logic has a flaw: when an error occurs in pure formatters (errno is set at line 163-168), the code continues to the buffer update section (line 176) which checks for errno and reports the error. However, after reporting the error and returning at line 188, the code then checks errno again at line 208 and returns again. This second check at line 208 is unreachable because the earlier return at line 188 has already exited. This creates confusion in the control flow.

Suggested change
if errno then
return
end

Copilot uses AI. Check for mistakes.

-- Apply impure formatters sequentially
local impure_error = nil
for _, config in ipairs(impure) do
local err = apply_impure_formatter(buf, config, fname, cwd)
if err then
fail(err)
return
-- Eval dynamic args
local cfg = vim.tbl_extend('force', {}, config)
if type(cfg.args) == 'function' then
cfg.args = cfg.args(buf)
end

local result = async.await(1, function(callback)
vim.system(util.get_cmd(cfg, fname, buf), {
text = true,
cwd = cwd,
env = cfg.env or {},
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

The impure formatter execution is missing the timeout parameter that was present in the original code. This could lead to formatters hanging indefinitely if they don't complete. The timeout should be added to the vim.system call options.

Suggested change
env = cfg.env or {},
env = cfg.env or {},
timeout = cfg.timeout or 5000,

Copilot uses AI. Check for mistakes.
}, callback)
end)

if result.code ~= 0 and #result.stderr > 0 then
impure_error = {
cmd = cfg.cmd,
code = result.code,
stderr = result.stderr,
}
break
end
end

-- Refresh buffer if impure formatters were used
if impure_error then
fail(
('%s exited with code %d\n%s'):format(
impure_error.cmd,
impure_error.code,
impure_error.stderr
)
)
return
end

if #impure > 0 then
async.await(1, function(callback)
vim.schedule(function()
api.nvim_buf_call(buf, function()
local views = save_views(buf)
api.nvim_command('silent! edit!')
restore_views(views)
end)
callback()
vim.schedule(function()
api.nvim_buf_call(buf, function()
local views = save_views(buf)
api.nvim_command('silent! edit!')
restore_views(views)
Comment on lines +251 to +255
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

The final vim.schedule call for impure formatters is missing the async.await wrapper. This means the code won't wait for the buffer refresh to complete before emitting the 'done' event. This could cause a race condition where the 'done' event fires before the buffer is actually refreshed. The original code wrapped this in async.await(1, function(callback) ... callback() end) to ensure proper sequencing.

Suggested change
vim.schedule(function()
api.nvim_buf_call(buf, function()
local views = save_views(buf)
api.nvim_command('silent! edit!')
restore_views(views)
async.await(1, function(callback)
vim.schedule(function()
api.nvim_buf_call(buf, function()
local views = save_views(buf)
api.nvim_command('silent! edit!')
restore_views(views)
end)
callback()

Copilot uses AI. Check for mistakes.
end)
end)
end

emit_event('done')
util.doau('GuardFmt', {
status = 'done',
})

if util.getopt('refresh_diagnostic') then
vim.diagnostic.show()
end
end)
end

M.do_fmt = do_fmt

return M
return {
do_fmt = do_fmt,
}
Loading