nvim-treesitter / nvim-treesitter-refactor

Refactor module for nvim-treesitter
Apache License 2.0
398 stars 25 forks source link

goto_next_usage doesn't work from several days ago #10

Closed owarai closed 3 years ago

owarai commented 3 years ago

Describe the bug

the feature of treesitter refactor part: goto_next_usage not work from several days ago, but goto_prev_usage still works.

I don't know if it's appropriate to post here, but yesterday I post the same issue on repo: nvim-treesitter-refactor, no response received

To Reproduce Steps to reproduce the behavior:

  1. packer.nvim add repos: 'nvim-treesitter/nvim-treesitter', 'nvim-treesitter/nvim-treesitter-textobjects', 'nvim-treesitter/nvim-treesitter-refactor'
  2. config it like:
  require('nvim-treesitter.configs').setup {
    -- enable markdown will cause crash: https://github.com/nvim-treesitter/nvim-treesitter/issues/602
    ensure_installed = wanted_parsers,
    highlight = {
      enable = true, -- false will disable the whole extension
      use_languagetree = false, -- Use this to enable language injection (this is very unstable)
      disable = {'elm'},
      custom_captures = { -- mapping of user defined captures to highlight groups
        -- ["foo.bar"] = "Identifier"   -- highlight own capture @foo.bar with highlight group "Identifier", see :h nvim-treesitter-query-extensions
      },
    },
    -- ......
    refactor = {
      highlight_definitions = {enable = true},
      highlight_current_scope = {enable = false},
      -- Renames the symbol under the cursor within the current scope (and current file).
      smart_rename = {enable = true, keymaps = {smart_rename = 'grr'}},
      navigation = {
        enable = true,
        keymaps = {
          goto_definition = 'gnd', -- mapping to go to definition of symbol under cursor
          list_definitions = 'gnD', -- mapping to list all definitions in current file
          list_definitions_toc = 'gO',
          goto_next_usage = '<a-*>',
          goto_previous_usage = '<a-#>',
        },
      },
    },
  }

Expected behavior goto_next_usage works like before.

Output of :checkhealth nvim_treesitter

======================================================================== ## Installation - OK: `git` executable found. - OK: `cc` executable found. ## lua parser healthcheck - OK: lua parser found. - OK: `highlights.scm` found. - OK: `locals.scm` found. - OK: `folds.scm` found. - OK: `indents.scm` found. ## go parser healthcheck - OK: go parser found. - OK: `highlights.scm` found. - OK: `locals.scm` found. - OK: `folds.scm` found. - WARNING: No `indents.scm` query found for go - ADVICE: - Open an issue at https://github.com/nvim-treesitter/nvim-treesitter ## rust parser healthcheck - OK: rust parser found. - OK: `highlights.scm` found. - OK: `locals.scm` found. - OK: `folds.scm` found. - WARNING: No `indents.scm` query found for rust - ADVICE: - Open an issue at https://github.com/nvim-treesitter/nvim-treesitter

Output of nvim --version

NVIM v0.5.0-dev+1025-g0fad952a2

Additional context repo & parsers all updated to latest.

vigoux commented 3 years ago

Hi, I am moving your issue in the correct repo, as this is a separate module.

vigoux commented 3 years ago

Could you try to bisect (using git bisect) when this started happening ? This will greatly help us !

owarai commented 3 years ago

Since this repo doesn't commit frequently, I rollback to commit 130d94(Sep 19, 2020), nothing different happened. I also rollback the commit of nvim-treesitter serveral times, stop onto ac4219(Dec 22, 2020), same with above.

goto_next_usage is a feature that I use frequently, so I think it can't have stopped working before this time point and I didn't notice it.

I tried to use goto_next_usage in lua/go/rust, but none of them worked.

The phenomenon: If I put my cursor to a variable name, trigger goto_next_usage will only move the cursor to the start of variable name.

owarai commented 3 years ago

By the way, does the goto_next_usage function work properly for you?

bapjiws commented 3 years ago

Had the same issue, installing it from commit 130d94 did help 🙂

harrygallagher4 commented 3 years ago

Just noticed this. I think it's just a math error in navigation.lua:146. The call below correctly finds the next usage:

require'nvim-treesitter-refactor.navigation'.goto_adjacent_usage(#buf, 2)

https://github.com/nvim-treesitter/nvim-treesitter-refactor/blob/16bbe963d044ec94316679868e0988caa7b5b4c3/lua/nvim-treesitter-refactor/navigation.lua#L146

harrygallagher4 commented 3 years ago

I looked into this more but I'm not particularly familiar with the internals of treesitter so I may be wrong about some things. It seems like find_usages returns a table with two copies of each node, or maybe two different nodes for each usage which both have the same row/column. index_of finds the first "copy" so subtracting 1 takes us to the second "copy" of the previous usage while adding one takes us to the second "copy" of the usage our cursor is currently on thus the cursor doesn't move.

Changing the definitions of goto_next_usage & goto_previous_usage to the following works:

function M.goto_next_usage(bufnr) return M.goto_adjacent_usage(bufnr, 2) end
function M.goto_previous_usage(bufnr) return M.goto_adjacent_usage(bufnr, -2) end

edit: it would probably make more sense to multiply delta by two in goto_adjacent_usage

theHamsta commented 3 years ago

@harrygallagher4 this is a upstream problem that affects all tree-sitter modules, see #11 in this repo or my fix for nvim-dap-virtual-text. Neovim right now just includes the queries multiple times (all query results are doubled) which means that it will think that the definition on which you currently are is the same position as the next position.

@vigoux, @steelsojka

rodamaral commented 3 years ago

By the way, in some filetypes, tthe results aren't doubled, they are multiplied by 8. So, using M.goto_adjacent_usage(bufnr, 2) is not reliable.

theHamsta commented 3 years ago

This is because the duplication is the result on how Neovim finds the query files. Depending on the filetype and your runtime path it may find one and the same file more than once and just appends them. Which results in doubling, trippling the reference results or just having them simple.

theHamsta commented 3 years ago

See this PR here: https://github.com/neovim/neovim/pull/13778 We could also fixing it be deduplicating the references in nvim-treesitter but this feels a bit like a hack. With #11 I fixed the same problem.

theHamsta commented 3 years ago

neovim/neovim#13778 has been merged. At least under normal circumstances (unless you have doubled-queries), this bug should no longer occur.

owarai commented 3 years ago

Confirmed that my issue from comment https://github.com/nvim-treesitter/nvim-treesitter-refactor/issues/10#issue-786707514 is fixed.

stsewd commented 3 years ago

@owarai can this be closed then?

owarai commented 3 years ago

Yes!!! thanks for your work on this.