From 00a528818463b10d84699b2e0f4a960d5a4aeb5c Mon Sep 17 00:00:00 2001 From: Steven Arcangeli Date: Tue, 12 Sep 2023 20:10:05 -0700 Subject: fix: modify diff calculation to handle end-of-file newlines better (#35) --- lua/conform/log.lua | 7 ++++++ lua/conform/runner.lua | 66 +++++++++++++++++++++++++++++--------------------- tests/fuzzer_spec.lua | 13 +++++----- tests/runner_spec.lua | 15 +++++------- tests/test_util.lua | 5 ++++ 5 files changed, 64 insertions(+), 42 deletions(-) diff --git a/lua/conform/log.lua b/lua/conform/log.lua index 3e31fd2..20d91e1 100644 --- a/lua/conform/log.lua +++ b/lua/conform/log.lua @@ -83,6 +83,13 @@ local function initialize() end end +---Override the file handler e.g. for tests +---@param handler fun(line: string) +function Log.set_handler(handler) + write = handler + initialized = true +end + function Log.log(level, msg, ...) if Log.level <= level then initialize() diff --git a/lua/conform/runner.lua b/lua/conform/runner.lua index a438718..ff8fbef 100644 --- a/lua/conform/runner.lua +++ b/lua/conform/runner.lua @@ -150,7 +150,7 @@ local function create_text_edit( -- If we're inserting text, make sure the text includes a newline at the end. -- The one exception is if we're inserting at the end of the file, in which case the newline is -- implicit - if is_insert and start_line < #original_lines - 1 then + if is_insert and start_line < #original_lines then table.insert(replacement, "") end local new_text = table.concat(replacement, "\n") @@ -180,25 +180,26 @@ M.apply_format = function(bufnr, original_lines, new_lines, range, only_apply_ra return end local bufname = vim.api.nvim_buf_get_name(bufnr) - -- If the formatter output didn't have a trailing newline, add one - if new_lines[#new_lines] ~= "" then - table.insert(new_lines, "") - end - - -- Vim buffers end with an implicit newline, so append an empty line to stand in for that - if vim.bo[bufnr].eol then - table.insert(original_lines, "") - end + log.trace("Applying formatting to %s", bufname) + -- The vim.diff algorithm doesn't handle changes in newline-at-end-of-file well. The unified + -- result_type has some text to indicate that the eol changed, but the indices result_type has no + -- such indication. To work around this, we just add a trailing newline to the end of both the old + -- and the new text. + table.insert(original_lines, "") + table.insert(new_lines, "") local original_text = table.concat(original_lines, "\n") local new_text = table.concat(new_lines, "\n") - log.trace("Creating diff for %s", bufname) + table.remove(original_lines) + table.remove(new_lines) + + log.trace("Comparing lines %s and %s", original_lines, new_lines) local indices = vim.diff(original_text, new_text, { result_type = "indices", algorithm = "histogram", }) assert(indices) + log.trace("Diff indices %s", indices) local text_edits = {} - log.trace("Creating TextEdits for %s", bufname) for _, idx in ipairs(indices) do local orig_line_start, orig_line_count, new_line_start, new_line_count = unpack(idx) local is_insert = orig_line_count == 0 @@ -232,7 +233,7 @@ M.apply_format = function(bufnr, original_lines, new_lines, range, only_apply_ra end end - log.trace("Applying text edits for %s", bufname) + log.trace("Applying text edits: %s", text_edits) vim.lsp.util.apply_text_edits(text_edits, bufnr, "utf-8") log.trace("Done formatting %s", bufname) end @@ -272,12 +273,14 @@ local function run_formatter(bufnr, formatter, config, ctx, input_lines, callbac log.info("Run %s on %s", formatter.name, vim.api.nvim_buf_get_name(bufnr)) local buffer_text -- If the buffer has a newline at the end, make sure we include that in the input to the formatter - if vim.bo[bufnr].eol then + local add_extra_newline = vim.bo[bufnr].eol + if add_extra_newline then table.insert(input_lines, "") - buffer_text = table.concat(input_lines, "\n") + end + log.trace("Input lines: %s", input_lines) + buffer_text = table.concat(input_lines, "\n") + if add_extra_newline then table.remove(input_lines) - else - buffer_text = table.concat(input_lines, "\n") end if not config.stdin then @@ -317,18 +320,27 @@ local function run_formatter(bufnr, formatter, config, ctx, input_lines, callbac stderr = data end, on_exit = function(_, code) - local output - if not config.stdin then - local fd = assert(uv.fs_open(ctx.filename, "r", 448)) -- 0700 - local stat = assert(uv.fs_fstat(fd)) - local content = assert(uv.fs_read(fd, stat.size)) - uv.fs_close(fd) - output = vim.split(content, "\n", { plain = true }) - else - output = stdout - end if vim.tbl_contains(exit_codes, code) then + local output + if not config.stdin then + local fd = assert(uv.fs_open(ctx.filename, "r", 448)) -- 0700 + local stat = assert(uv.fs_fstat(fd)) + local content = assert(uv.fs_read(fd, stat.size)) + uv.fs_close(fd) + output = vim.split(content, "\n", { plain = true }) + else + output = stdout + end + -- Remove the trailing newline from the output to convert back to vim lines representation + if add_extra_newline and output[#output] == "" then + table.remove(output) + end + -- Vim will never let the lines array be empty. An empty file will still look like { "" } + if #output == 0 then + table.insert(output, "") + end log.debug("%s exited with code %d", formatter.name, code) + log.trace("Output lines: %s", output) callback(nil, output) else log.info("%s exited with code %d", formatter.name, code) diff --git a/tests/fuzzer_spec.lua b/tests/fuzzer_spec.lua index 81c032f..639cfa9 100644 --- a/tests/fuzzer_spec.lua +++ b/tests/fuzzer_spec.lua @@ -1,6 +1,7 @@ require("plenary.async").tests.add_to_env() local test_util = require("tests.test_util") local conform = require("conform") +local log = require("conform.log") local runner = require("conform.runner") describe("fuzzer", function() @@ -25,12 +26,6 @@ describe("fuzzer", function() vim.api.nvim_buf_set_lines(bufnr, 0, -1, true, buf_content) vim.bo[bufnr].modified = false runner.apply_format(0, buf_content, expected, nil, false) - -- We expect the last newline to be effectively "swallowed" by the formatter - -- because vim will use that as the EOL at the end of the file. The exception is that we always - -- expect at least one line in the output - if #expected > 1 and expected[#expected] == "" then - table.remove(expected) - end assert.are.same(expected, vim.api.nvim_buf_get_lines(0, 0, -1, false)) end @@ -95,6 +90,10 @@ describe("fuzzer", function() for _ = 1, num_lines do table.remove(lines, idx) end + -- vim will never let the lines be empty. An empty file has a single blank line. + if #lines == 0 then + table.insert(lines, "") + end end local function make_edits(lines) @@ -112,8 +111,10 @@ describe("fuzzer", function() end it("formats correctly", function() + -- log.level = vim.log.levels.TRACE for i = 1, 50000 do math.randomseed(i) + log.info("Fuzz testing with seed %d", i) local content = make_file(20) local formatted = make_edits(content) run_formatter(content, formatted) diff --git a/tests/runner_spec.lua b/tests/runner_spec.lua index 8807d2d..10ea075 100644 --- a/tests/runner_spec.lua +++ b/tests/runner_spec.lua @@ -125,12 +125,6 @@ describe("runner", function() local expected_lines = vim.split(expected, "\n", { plain = true }) test_util.set_formatter_output(expected_lines) conform.format(vim.tbl_extend("force", opts or {}, { formatters = { "test" }, quiet = true })) - -- We expect the last newline to be effectively "swallowed" by the formatter - -- because vim will use that as the EOL at the end of the file. The exception is that we always - -- expect at least one line in the output - if #expected_lines > 1 and expected_lines[#expected_lines] == "" then - table.remove(expected_lines) - end return expected_lines end @@ -193,11 +187,14 @@ print("a") run_formatter_test("\nfoo", "\nhello\nfoo") run_formatter_test("hello", "hello\n") run_formatter_test("hello", "hello\n\n") - run_formatter_test("hello", "hello\n") - -- This should generate no changes to the buffer - assert.falsy(vim.bo.modified) run_formatter_test("hello\n", "hello") run_formatter_test("hello\n ", "hello") + + -- These should generate no changes to the buffer + run_formatter_test("hello\n", "hello\n") + assert.falsy(vim.bo.modified) + run_formatter_test("hello", "hello") + assert.falsy(vim.bo.modified) end) it("does not change output if formatter fails", function() diff --git a/tests/test_util.lua b/tests/test_util.lua index a225aec..cded23a 100644 --- a/tests/test_util.lua +++ b/tests/test_util.lua @@ -1,5 +1,6 @@ require("plenary.async").tests.add_to_env() local conform = require("conform") +local log = require("conform.log") local M = {} local OUTPUT_FILE = "tests/fake_formatter_output" @@ -21,6 +22,8 @@ M.reset_editor = function() if vim.fn.filereadable(OUTPUT_FILE) == 1 then vim.fn.delete(OUTPUT_FILE) end + log.level = vim.log.levels.ERROR + log.set_handler(print) end ---@param lines string[] @@ -28,6 +31,8 @@ M.set_formatter_output = function(lines) local content = table.concat(lines, "\n") local fd = assert(vim.loop.fs_open(OUTPUT_FILE, "w", 420)) -- 0644 vim.loop.fs_write(fd, content) + -- Make sure we add the final newline + vim.loop.fs_write(fd, "\n") vim.loop.fs_close(fd) end -- cgit v1.2.3-70-g09d2