Skip to content
This repository has been archived by the owner on Aug 12, 2023. It is now read-only.

Managing undo history #879

Closed
1 task done
piersolenski opened this issue May 20, 2022 · 9 comments
Closed
1 task done

Managing undo history #879

piersolenski opened this issue May 20, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@piersolenski
Copy link

Issues

  • I have checked existing issues and there are no existing ones with the same request.

Feature description

Hey! I'm moving from Neoformat to null-ls. I used to use Neoformat with https://github.com/Pocco81/AutoSave.nvim. null-ls has a way to format on save https://github.com/jose-elias-alvarez/null-ls.nvim/wiki/Formatting-on-save#code. Each time a code change is made or the text changes, the save functionality is run. That means it's generally saved twice. So when you try to undo the last change, it's undoing to the last formatted rather than the actual code content changing. This means you need to undo twice, and it can be a bit visually jumpy whilst it's trying to format you go through the changes. Neoformat used to handle this like so, https://github.com/sbdchd/neoformat#managing-undo-history - is there a way to do this with null-ls? Thanks!

Help

No

Implementation help

No response

@piersolenski piersolenski added the enhancement New feature or request label May 20, 2022
@jose-elias-alvarez
Copy link
Owner

jose-elias-alvarez commented May 20, 2022

If you use a BufWritePre autocommand (the recommended approach), the file will only be saved once, but the issue of creating an undo point for formatting changes still remains. I don't personally consider this an issue, since I format often on save and do sometimes want to temporarily undo a formatter's changes.

The only way to do this that I'm aware of would be undojoin, but I'm not sure how undojoin interacts with the Lua API that Neovim uses to apply LSP formatting results. I tried creating a Vim command to wrap the formatting method (vim.lsp.buf.formatting_sync on 0.7, vim.lsp.buf.format on 0.8):

null_ls.setup({
    on_attach = function(client, bufnr)
        if client.supports_method("textDocument/formatting") then
            vim.api.nvim_buf_create_user_command(bufnr, "LspFormatting", function()
                vim.lsp.buf.format()
            end)
        end
    end,
})

This seems to work when I manually call :undojoin | LspFormatting but not via an autocommand, strangely. It might be worth playing around with this more, but if the underlying text modification APIs don't play nicely with undojoin then there's nothing we can really do here, since the problem is inherent to all LSP formatting and not null-ls specific (assuming you aren't using a custom handler).

@piersolenski
Copy link
Author

When you say you don't consider this an issue, with your setup do you have this jarring jumping effect - or is there a workaround for it?

Screenshot.2022-05-21.at.15.17.35-converted.mp4

For example, here I do di{, and then have to hit u twice in quick succession to avoid it trying to reformat the change.

My super hacky work around at the moment is:

if client.supports_method("textDocument/formatting") then
  vim.api.nvim_set_keymap("n", "u", "2u", { noremap = true })
end

😅

@jose-elias-alvarez
Copy link
Owner

I am not sure what your setup looks like, but are you automatically writing the file when you leave insert mode? That looks painful, and if I were going to do that, I would definitely not set up formatting on save. With the recommended setup - a simple BufWritePre autocommand - you won’t see this behavior.

@piersolenski
Copy link
Author

Yes, the default behaviour for most autosave plugins is to save on the InsertLeave event, alongside TextChanged; https://github.com/Pocco81/AutoSave.nvim#setup-configuration / https://github.com/907th/vim-auto-save#events. Once you stop ever having to hit save it becomes more painful to remember to have to! I guess it's especially nice when you're doing front end work with live reloading and you want to see changes instantaneously... It's the one thing I'm really missing from Neoformat as I love everything else about null-ls!

@piersolenski
Copy link
Author

piersolenski commented May 22, 2022

With https://github.com/Pocco81/AutoSave.nvim, formatting before saving via the hook_before_saving rather than via null-ls seems to work, albeit feeling slightly slower...

local autosave = require("autosave")

autosave.setup()

autosave.hook_before_saving = function()
	vim.lsp.buf.formatting_sync()
end

@jose-elias-alvarez
Copy link
Owner

jose-elias-alvarez commented May 22, 2022

I played around with AutoSave.nvim, and I think this works - if I make a change that requires the file to be formatted and then undo, it undoes both the formatting changes and my original change - but I'm not 100% sure what the expected behavior is, so I can't say for sure:

local augroup = vim.api.nvim_create_augroup("LspFormatting", {})

require("null-ls").setup({
    sources = sources,
    on_attach = function(client, bufnr)
        if client.supports_method("textDocument/formatting") then
            vim.api.nvim_buf_create_user_command(bufnr, "LspFormatting", function()
                -- or vim.lsp.buf.formatting(bufnr) on 0.8
                vim.lsp.buf.formatting_sync()
            end, {})

            -- you can leave this out if your on_attach is unique to null-ls,
            -- but if you share it with multiple servers, you'll want to keep it
            vim.api.nvim_clear_autocmds({
                group = augroup,
                buffer = bufnr,
            })

            vim.api.nvim_create_autocmd("BufWritePre", {
                group = augroup,
                buffer = bufnr,
                command = "undojoin | LspFormatting",
            })
        end
    end,
})

I also ran into this issue when manually saving the file, but it seems to work well enough with AutoSave.nvim, at least during my quick testing.

I have a better understanding of the use case now and am happy to investigate how we can support it. Having said that, I'll mention that null-ls is not doing anything special here - all it's doing is passing text document changes to Neovim's LSP handlers, so whatever issues null-ls has here will also exist with actual LSP formatting.

@piersolenski
Copy link
Author

That's awesome!

I've played around with the two methods... the first has the advantage of there being no split-second view of the unformatted code, but it feels a tiny but perceivable bit slower.

Your method feels snappier, although you do get the slightly disorientating view of the unformatted code - although I'm used to it personally.

I prefer the faster implementation because not all text editing requires formatting anyway, so it's not worth slowing everything down for simple text changes, but leaving this comparison here for anyone else who might prefer the other method.

Thanks again!

@jose-elias-alvarez
Copy link
Owner

I added a link to the workaround to the wiki, so I'm going to close this.

@hanxi
Copy link

hanxi commented Dec 11, 2022

Thanks! I solved it. I use nvim0.8 .

local augroup = vim.api.nvim_create_augroup("LspFormatting", {})
local null_ls = require("null-ls")
null_ls.setup({
    sources = {
        null_ls.builtins.formatting.gofmt,
    },
    on_attach = function(client, bufnr)
        if client.supports_method("textDocument/formatting") then
            vim.api.nvim_buf_create_user_command(bufnr, "LspFormatting", function ()
                vim.lsp.buf.format({ bufnr = bufnr })
            end, {})
            vim.api.nvim_clear_autocmds({ group = augroup, buffer = bufnr })
            vim.api.nvim_create_autocmd("BufWritePre", {
                group = augroup,
                buffer = bufnr,
                command = "undojoin | LspFormatting",
            })
        end
    end,
})

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants