conform.nvim icon indicating copy to clipboard operation
conform.nvim copied to clipboard

bug: Cursor can jump when more than one lsp provides text formatting

Open abeldekat opened this issue 2 years ago • 3 comments

Neovim version (nvim -v)

0.9.4

Operating system/version

Arch linux

Add the debug logs

  • [X] I have set log_level = vim.log.levels.DEBUG and pasted the log contents below.

Log file

Log file: /home/user/.config/nvim/.repro//state/nvim/conform.log 20:17:06[DEBUG] Running LSP formatter on /home/user/.config/nvim/test.sv 20:17:06[DEBUG] Converting full-file LSP format to piecewise format 20:17:07[DEBUG] Converting full-file LSP format to piecewise format

Formatters for this buffer: LSP: verible, svlangserver

Describe the bug

https://github.com/stevearc/conform.nvim/assets/58370433/77fe25d5-4637-4586-932a-739a3b67c783

Based on this issue I extracted a reproducible scenario where the cursor jumps when using conform.nvim.

There are two language servers active, both can format. LazyVim calls conform.nvim with opts.lsp_fallback = true.

When conform.nvim is disabled, LazyVim calls vim.lsp.buf.format directly, and the cursor does not jump.

Also, when both language servers result in the same output, the cursor does not jump.

lazyvim.util.lsp.lua:

function M.format(opts)
  opts = vim.tbl_deep_extend("force", {}, opts or {}, require("lazyvim.util").opts("nvim-lspconfig").format or {})
  local ok, conform = pcall(require, "conform")
  -- use conform for formatting with LSP when available,
  -- since it has better format diffing
  if ok then
    opts.formatters = {}
    opts.lsp_fallback = true
    conform.format(opts)
  else
    vim.lsp.buf.format(opts)
  end
end

Steps To Reproduce

  1. nvim -u repro.lua test.sv, restart when installation complete
  2. place cursor on line 149 (always begin) or inside any task
  3. press :w

Expected Behavior

The cursor does not jump

Minimal example file

click to open test.sv
`timescale 1ns / 1ps

import params_pkg::*;

module MVM_tb ();

    parameter CLK100_PEROID = 10;
    parameter CLK200_PEROID = 5;

    bit clk, clk_ddr;
    bit rst;  //active high
    logic [IC_N-1:0] strobe_i;
    logic load_mode_i;
    logic signed [WINDOW_SIZE-1:0][DATA_WIDTH-1:0] vector_i[IC_N];
    logic valid_i;
    logic ready_i;
    logic valid_o;
    logic ready_o;
    logic signed [DATA_WIDTH-1:0] mvm_o[OC_N];
    logic signed [WINDOW_SIZE-1:0][DATA_WIDTH-1:0] weights_record[WEIGHT_CYCLES][IC_N];
    logic signed [WINDOW_SIZE-1:0][DATA_WIDTH-1:0] vector_record[CALC_CYCLES][IC_N];
    logic signed [DATA_WIDTH-1:0] result[CALC_CYCLES][OC_N];
    integer seed;
    int ic_idx, wd_idx;
    int wght_cycle, calc_cycle;
    int cur_calc_cycle;

    wire valid;
    wire signed [DATA_WIDTH-1:0] data[OC_N];
    assign valid = MVM_inst.funnel_inst_7.valid_o;
    assign data  = MVM_inst.funnel_inst_7.data_o;

    MVM_if mvm_if (
        .clk(clk),
        .clk_ddr(clk_ddr),
        .rst(rst)
    );

    MVM MVM_inst (.*);

    initial begin
        seed = 5;
        rst  = 1;
        #200;
        rst = 0;
    end

    initial begin
        clk = 0;
        forever begin
            #(CLK100_PEROID / 2) clk = ~clk;
        end
    end
    initial begin
        clk_ddr = 0;
        forever begin
            #(CLK200_PEROID / 2) clk_ddr = ~clk_ddr;
        end
    end

    initial begin
        $fsdbDumpfile("./wave/wave.fsdb");
        $fsdbDumpvars();
        $fsdbDumpMDA();
    end

    always_ff @(posedge clk) begin
        if (rst)
            for (int i = 0; i < WEIGHT_CYCLES; i++) begin
                for (int j = 0; j < IC_N; j++) begin
                    weights_record[i][j] <= '0;
                end
            end
        else if (valid_i & (~load_mode_i)) begin
            for (int i = 1; i < WEIGHT_CYCLES; i++) begin
                weights_record[i] <= weights_record[i-1];
            end
            weights_record[0] <= vector_i;
        end
    end
    always_ff @(posedge clk) begin
        if (rst)
            for (int i = 0; i < CALC_CYCLES; i++) begin
                for (int j = 0; j < IC_N; j++) begin
                    vector_record[i][j] <= '0;
                end
            end
        else if (valid_i & load_mode_i) begin
            for (int i = 0; i < CALC_CYCLES - 1; i++) begin
                vector_record[i] <= vector_record[i+1];
            end
            vector_record[CALC_CYCLES-1] <= vector_i;
        end
    end

    initial begin
        ready_i <= 1'b1;
        strobe_i <= {IC_N{1'b0}};
        load_mode_i <= 1'b0;
        for (int i = 0; i < 8; i++) begin : VECTOR_INIT
            vector_i[i] <= {VECTOR_IC_WIDTH{1'b0}};
        end
        valid_i <= 1'b0;
        #300;

        for (wght_cycle = 0; wght_cycle < WEIGHT_CYCLES; wght_cycle++) begin : DELIVER_WEIGHTS
            @(posedge clk);
            for (ic_idx = 0; ic_idx < IC_N; ic_idx++) begin : VECTOR_IC_ASSIGN
                for (wd_idx = 0; wd_idx < WINDOW_SIZE; wd_idx++) begin : VECTOR_WIDNDOW_ASSIGN
                    vector_i[ic_idx][wd_idx] <= {$urandom()} % 6;
                end
            end
            strobe_i <= {IC_N{1'b1}};
            load_mode_i <= 1'b0;
            valid_i <= 1'b1;
            seed <= seed + 1;
        end
        disable_deliver_weights();
        for (int i = 0; i < 3; i++) begin
            @(posedge clk);
        end

        for (int test_iter = 0; test_iter < 5; test_iter++) begin : TEST_TIMES
            cur_calc_cycle = $urandom_range(CALC_CYCLES, 1);
            $display("This time calculation cycles: %d\n", cur_calc_cycle);
            for (
                calc_cycle = 0; calc_cycle < cur_calc_cycle; calc_cycle++
            ) begin : DELIVER_VECTOR_CALC_1st
                @(posedge clk);
                for (ic_idx = 0; ic_idx < IC_N; ic_idx++) begin : VECTOR_IC_ASSIGN
                    for (wd_idx = 0; wd_idx < WINDOW_SIZE; wd_idx++) begin : VECTOR_WIDNDOW_ASSIGN
                        vector_i[ic_idx][wd_idx] <= {$urandom()} % 6;
                    end
                end
                strobe_i <= {IC_N{1'b1}};
                load_mode_i <= 1'b1;
                valid_i <= 1'b1;
                seed <= seed + 1;
            end
            disable_deliver_calc();
            #500;
        end

        #2000;

        $finish;
    end

    always begin
        validate_result();
    end

    task automatic disable_deliver_weights();
        @(posedge clk);
        strobe_i <= {IC_N{1'b0}};
        load_mode_i <= 1'b0;
        valid_i <= 1'b0;
    endtask

    task automatic disable_deliver_calc();
        @(posedge clk);
        strobe_i <= {IC_N{1'b0}};
        load_mode_i <= 1'b0;
        valid_i <= 1'b0;
    endtask

    task automatic validate_result();
        int cycle_index;
        wait (valid == 1'b1);
        cycle_index = CALC_CYCLES - cur_calc_cycle;
        calculate();
        $display("\nMVM output valid!\n");
        @(negedge clk);
        while (valid != 1'b0) begin
            for (int i = 0; i < OC_N; i++) begin
                if (result[cycle_index][i] != data[i]) begin
                    $display("\n Test fail!!!\n");
                    $display("current time: %t\n", $realtime);
                end
            end
            @(negedge clk);
            cycle_index++;
        end
    endtask

    task automatic calculate();
        int res_temp = 0;
        for (int cycle = 0; cycle < CALC_CYCLES; cycle++) begin
            for (int o = 0; o < OC_N; o++) begin
                res_temp = 0;
                for (int ic = 0; ic < IC_N; ic++) begin
                    for (int wd = 0; wd < WINDOW_SIZE; wd++) begin
                        res_temp += weights_record[o][ic][wd] * vector_record[cycle][ic][wd];
                    end
                end
                result[cycle][o] = res_temp;
            end
        end
    endtask

endmodule

Minimal init.lua

local function bootstrap(root) -- DO NOT change the paths
  for _, name in ipairs({ "config", "data", "state", "cache" }) do
    vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
  end
  local lazypath = root .. "/plugins/lazy.nvim"
  if not vim.loop.fs_stat(lazypath) then
    vim.fn.system({ "git", "clone", "--filter=blob:none", "https://github.com/folke/lazy.nvim.git", lazypath })
  end
  vim.opt.rtp:prepend(vim.env.LAZY or lazypath)
end
local root = vim.fn.fnamemodify("./.repro", ":p")
bootstrap(root)

-- recognize systemverilog
vim.api.nvim_create_autocmd({ "BufEnter", "BufAdd", "BufRead" }, {
  pattern = { "*.vh", "*.svh" },
  command = "set filetype=systemverilog",
})

local plugins = {
  {
    "abeldekat/lazyflex.nvim",
    version = "*",
    import = "lazyflex.hook",
    opts = {
      lazyvim = { presets = { "colorscheme", "lsp", "formatting" } },
    },
  },
  { "LazyVim/LazyVim", import = "lazyvim.plugins" },
  {
    "neovim/nvim-lspconfig",
    opts = {
      servers = {
        verible = {},
        svlangserver = {
          cmd = { "svlangserver" },
          filetypes = { "verilog", "systemverilog" },
          settings = {
            systemverilog = {
              includeIndexing = { "*.{v,vh,sv,svh}", "**/*.{v,vh,sv,svh}" },
              disableLinting = true,
              disableCompletionProvider = true,
              disableHoverProvider = false,
              disableSignatureHelpProvider = true,
              -- > no cursor jumping when indentation_spaces is unspecified
              formatCommand = "verible-verilog-format --indentation_spaces=4",
            },
          },
          single_file_support = true,
        },
      },
    },
  },
  -- > set enabled = false to disable conform.nvim
  { "conform.nvim", enabled = true, opts = { log_level = vim.log.levels.DEBUG } },
}
require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

Additional context

No response

abeldekat avatar Nov 24 '23 19:11 abeldekat

I don't think it's very accurate to say mulltiple LSP. I think it's because of multiple formatter for one buffer. If there are two LSP for one buffer but only one with formatting feature, I guess that's OK.

beyond-fu avatar Nov 25 '23 13:11 beyond-fu

Thanks @beyond-fu

How about: Cursor can jump when more than one lsp provides text formatting

abeldekat avatar Nov 25 '23 14:11 abeldekat

@abeldekat I think it's OK.

beyond-fu avatar Nov 25 '23 14:11 beyond-fu

@beyond-fu Is this still an issue in your workflow?

@stevearc, I would not mind if this issue were to be closed.

abeldekat avatar Aug 02 '24 20:08 abeldekat