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

Jump using ctrl-o not correct after navigating with lsp picker

Open abeldekat opened this issue 1 year ago • 6 comments

Contributing guidelines

Module(s)

mini.pick, mini.extra

Description

https://github.com/echasnovski/mini.nvim/assets/58370433/c5362c94-b91c-44f9-822b-f0b06b999696

When using lsp definition, I often jump back with ctrl-o. The jump is not correct. I also notice this behavior when using lsp references

Neovim version

0.10

Steps to reproduce


--[[
Use:
  mkdir ~/.config/repro
  cd ~/.config/repro

  touch init.lua
  add the contents of this file to init.lua
  NVIM_APPNAME=repro nvim init.lua 

Remove:
  rm -rf ~/.local/share/repro ~/.local/state/repro ~/.local/cache/repro
  rm -rf ~/.config/repro
--]]

local verify_with_fzf_lua = false -- set to true to use fzf-lua

local function clone(path_to_site)
  local mini_path = path_to_site .. "pack/deps/start/mini.nvim"
  if not vim.loop.fs_stat(mini_path) then
    vim.cmd('echo "Installing `mini.nvim`" | redraw')
    local clone_cmd =
      { "git", "clone", "--filter=blob:none", "https://github.com/echasnovski/mini.nvim", mini_path }
    vim.fn.system(clone_cmd)
    vim.cmd("packadd mini.nvim | helptags ALL")
    vim.cmd('echo "Installed `mini.nvim`" | redraw')
  end
end

local path_to_site = vim.fn.stdpath("data") .. "/site/"
clone(path_to_site)
local MiniDeps = require("mini.deps")
MiniDeps.setup({ path = { package = path_to_site } })
local add, now = MiniDeps.add, MiniDeps.now

vim.g.mapleader = " "
vim.opt.number = true

local function lua_ls_handler()
  require("lspconfig").lua_ls.setup({
    settings = {
      Lua = {
        runtime = { version = "LuaJIT" },
        workspace = {
          checkThirdParty = false,
          library = {
            vim.env.VIMRUNTIME,
            "${3rd}/luv/library",
          },
        },
      },
    },
  })
end

now(function()
  vim.cmd("colorscheme randomhue")

  add("nvim-treesitter/nvim-treesitter")
  ---@diagnostic disable-next-line: missing-fields
  require("nvim-treesitter.configs").setup({
    ensure_installed = { "lua" },
  })

  add("williamboman/mason.nvim")
  require("mason-registry"):on("package:install:success", function()
    vim.defer_fn(function()
      vim.cmd("LspStart")
    end, 100)
  end)
  require("mason").setup()

  add("williamboman/mason-lspconfig.nvim")
  add("neovim/nvim-lspconfig")
  require("mason-lspconfig").setup({
    ensure_installed = { "lua_ls" },
    handlers = {
      lua_ls = lua_ls_handler,
    },
  })

  local cmd_definition = "<cmd>Pick lsp scope='definition'<cr>"
  local cmd_references = "<cmd>Pick lsp scope='references'<cr>"
  if verify_with_fzf_lua then
    cmd_definition = "<cmd>FzfLua lsp_definitions<cr>"
    cmd_references = "<cmd>FzfLua lsp_references<cr>"
  end
  vim.api.nvim_create_autocmd("LspAttach", {
    callback = function(args)
      vim.keymap.set("n", "gd", cmd_definition, { buffer = args.buf, silent = true, desc = "Goto definition" })
      vim.keymap.set("n", "gr", cmd_references, { buffer = args.buf, silent = true, desc = "References" })
    end,
  })

  add("ibhagwan/fzf-lua") -- use by setting verify_with_fzf_lua to true
  require("fzf-lua").setup()
  require("mini.extra").setup()
  require("mini.pick").setup()
end)

Steps using the repro above:

  1. Open neovim: repro init.lua
  2. :30, selecting the line with "clone"
  3. press gd and enter
  4. line 17 is selected
  5. press ctrl-o
  6. cursor jumps to line 1 instead of line 30

Close neovim and change the following:

verify_with_fzf_lua = true

Repeat steps 1 till 6. The cursor correctly jumps to line 30.

Expected behavior

When I jump back after using Pick lsp scope="definition", the jump is correct.

Actual behavior

Jump is incorrect.

abeldekat avatar Jun 19 '24 09:06 abeldekat

Thanks for the suggestion!

I can reproduce that indeed MiniExtra.pickers.lsp does not add previous position to jumplist. After quick look, I didn't find a quick fix, though. I'll take a closer look.

echasnovski avatar Jun 19 '24 09:06 echasnovski

@echasnovski,

I noticed that you changed the type of the issue from "bug" into "feature-request". Was my assumption incorrect?

abeldekat avatar Jun 19 '24 09:06 abeldekat

I think it is reasonable to consider a bug as a deviation from documented and/or assumed during implementation behavior.

In this particular case adding current position to jumplist is neither.

echasnovski avatar Jun 19 '24 10:06 echasnovski

I understand. Thanks!

abeldekat avatar Jun 19 '24 13:06 abeldekat

Should this also work with ctrl-t (tagstack) ? I'm getting E73: Tag stack empty

samuel-andres avatar Aug 21 '24 04:08 samuel-andres

Should this also work with ctrl-t (tagstack) ? I'm getting E73: Tag stack empty

I don't think it should work because jumping with 'lsp' picker doesn't contribute to tag stack (hence the empty tag stack error).

echasnovski avatar Aug 21 '24 08:08 echasnovski

Is this being looked at or is it intended behaviour? It's the one thing keeping me from using mini.pick rather than telescope.nvim

If it is open, then it is a part of plan to have this resolved. Otherwise it would have been closed.

echasnovski avatar Oct 18 '24 13:10 echasnovski

Looking at the way telescope.nvim handles lsp calls, they seem to add the location using the following:

vim.cmd "normal! m'"

Specifically for pickers from LSP type sources. Perhaps this would be useful.

SimonYde avatar Oct 18 '24 20:10 SimonYde

This now should be resolved on latest main (should have been autoclosed via https://github.com/echasnovski/mini.nvim/commit/26a64563cfeeecb52bb6bfe8beda8493bf690c4e but I mistyped the issue number; oh well :( ).

I realized that probably the main friction point for me picking this issue to resolve was that I didn't understand that this doesn't work when jumping in the same buffer. I remember testing it (after the initial confirm) in my config with MiniExtra.pickers.lsp which lead to a different buffer and pressing <C-o> worked. So I concluded that it is something non-trivial and postponed. Modifying the default_choose to force updating jumplist was indeed the answer here to make it work even with jump to the same buffer.

echasnovski avatar Feb 01 '25 15:02 echasnovski

Thanks!

abeldekat avatar Feb 02 '25 08:02 abeldekat