Merge pull request #14589 from mhinz/revert-smarter-tagstack

Unfortunately, there are some subtle bugs in the smarter tagstack changes,
so we'll revert them for now and try to come up with a better approach.

One of the added tests, adds current position to jumplist before jumping,
is valuable though and changed to still work after reverting the other two
commits.

Closes #14571

runtime/lua/vim/lsp/util.lua

@@ -914,23 +914,6 @@ function M.make_floating_popup_options(width, height, opts)
   }
 end
 
-local function _should_add_to_tagstack(new_item)
-  local stack = vim.fn.gettagstack()
-
-  -- Check if we're at the bottom of the tagstack.
-  if stack.curidx <= 1 then return true end
-
-  local top_item = stack.items[stack.curidx-1]
-
-  -- Check if the item at the top of the tagstack is exactly the
-  -- same as the one we want to push.
-  if top_item.tagname ~= new_item.tagname then return true end
-  for i, v in ipairs(top_item.from) do
-    if v ~= new_item.from[i] then return true end
-  end
-  return false
-end
-
 --- Jumps to a location.
 ---
 --@param location (`Location`|`LocationLink`)
@@ -939,36 +922,22 @@ function M.jump_to_location(location)
   -- location may be Location or LocationLink
   local uri = location.uri or location.targetUri
   if uri == nil then return end
-
+  local bufnr = vim.uri_to_bufnr(uri)
-  local from_bufnr = vim.fn.bufnr('%')
-  local from = {from_bufnr, vim.fn.line('.'), vim.fn.col('.'), 0}
-  local item = {tagname=vim.fn.expand('<cword>'), from=from}
-
   -- Save position in jumplist
-  vim.cmd("mark '")
+  vim.cmd "normal! m'"
+
+  -- Push a new item into tagstack
+  local from = {vim.fn.bufnr('%'), vim.fn.line('.'), vim.fn.col('.'), 0}
+  local items = {{tagname=vim.fn.expand('<cword>'), from=from}}
+  vim.fn.settagstack(vim.fn.win_getid(), {items=items}, 't')
 
   --- Jump to new location (adjusting for UTF-16 encoding of characters)
-  local bufnr = vim.uri_to_bufnr(uri)
   api.nvim_set_current_buf(bufnr)
   api.nvim_buf_set_option(0, 'buflisted', true)
   local range = location.range or location.targetSelectionRange
   local row = range.start.line
   local col = get_line_byte_from_position(0, range.start)
-  -- This prevents the tagstack to be filled with items that provide
-  -- no motion when CTRL-T is pressed because they're both the source
-  -- and the destination.
-  local motionless =
-    bufnr == from_bufnr and
-    row+1 == from[2] and col+1 == from[3]
-  if not motionless and _should_add_to_tagstack(item) then
-    local winid = vim.fn.win_getid()
-    local items = {item}
-    vim.fn.settagstack(winid, {items=items}, 't')
-  end
-
-  -- Jump to new location
   api.nvim_win_set_cursor(0, {row + 1, col})
-
   return true
 end
 

test/functional/plugin/lsp_spec.lua

@@ -9,6 +9,7 @@ local eq = helpers.eq
 local pcall_err = helpers.pcall_err
 local pesc = helpers.pesc
 local insert = helpers.insert
+local funcs = helpers.funcs
 local retry = helpers.retry
 local NIL = helpers.NIL
 local read_file = require('test.helpers').read_file
@@ -1820,36 +1821,20 @@ describe('LSP', function()
   end)
 
   describe('lsp.util.jump_to_location', function()
-    local default_target_bufnr
+    local target_bufnr
-    local default_target_uri = 'file://fake/uri'
-
-    local create_buf = function(uri, lines)
-      for i, line in ipairs(lines) do
-        lines[i] = '"' .. line .. '"'
-      end
-      lines = table.concat(lines, ", ")
-
-      -- Let's set "hidden" to true in order to avoid errors when switching
-      -- between buffers in test.
-      local code = string.format([[
-        vim.api.nvim_set_option('hidden', true)
-
-        local bufnr = vim.uri_to_bufnr("%s")
-        local lines = {%s}
-        vim.api.nvim_buf_set_lines(bufnr, 0, 1, false, lines)
-        return bufnr
-      ]], uri, lines)
-
-      return exec_lua(code)
-    end
 
     before_each(function()
-      default_target_bufnr = create_buf(default_target_uri, {'1st line of text', 'å å ɧ 汉语 ↥ 🤦 🦄'})
+      target_bufnr = exec_lua [[
+        local bufnr = vim.uri_to_bufnr("file://fake/uri")
+        local lines = {"1st line of text", "å å ɧ 汉语 ↥ 🤦 🦄"}
+        vim.api.nvim_buf_set_lines(bufnr, 0, 1, false, lines)
+        return bufnr
+      ]]
     end)
 
-    local location = function(uri, start_line, start_char, end_line, end_char)
+    local location = function(start_line, start_char, end_line, end_char)
       return {
-        uri = uri,
+        uri = "file://fake/uri",
         range = {
           start = { line = start_line, character = start_char },
           ["end"] = { line = end_line, character = end_char },
@@ -1857,9 +1842,9 @@ describe('LSP', function()
       }
     end
 
-    local jump = function(bufnr, msg)
+    local jump = function(msg)
       eq(true, exec_lua('return vim.lsp.util.jump_to_location(...)', msg))
-      eq(bufnr, exec_lua[[return vim.fn.bufnr('%')]])
+      eq(target_bufnr, exec_lua[[return vim.fn.bufnr('%')]])
       return {
         line = exec_lua[[return vim.fn.line('.')]],
         col = exec_lua[[return vim.fn.col('.')]],
@@ -1867,13 +1852,13 @@ describe('LSP', function()
     end
 
     it('jumps to a Location', function()
-      local pos = jump(default_target_bufnr, location(default_target_uri, 0, 9, 0, 9))
+      local pos = jump(location(0, 9, 0, 9))
       eq(1, pos.line)
       eq(10, pos.col)
     end)
 
     it('jumps to a LocationLink', function()
-      local pos = jump(default_target_bufnr, {
+      local pos = jump({
           targetUri = "file://fake/uri",
           targetSelectionRange = {
             start = { line = 0, character = 4 },
@@ -1889,104 +1874,22 @@ describe('LSP', function()
     end)
 
     it('jumps to the correct multibyte column', function()
-      local pos = jump(default_target_bufnr, location(default_target_uri, 1, 2, 1, 2))
+      local pos = jump(location(1, 2, 1, 2))
       eq(2, pos.line)
       eq(4, pos.col)
       eq('å', exec_lua[[return vim.fn.expand('<cword>')]])
     end)
 
     it('adds current position to jumplist before jumping', function()
-      exec_lua([[
+      funcs.nvim_win_set_buf(0, target_bufnr)
-        vim.api.nvim_win_set_buf(0, ...)
+      local mark = funcs.nvim_buf_get_mark(target_bufnr, "'")
-        vim.api.nvim_win_set_cursor(0, {2, 0})
+      eq({ 1, 0 }, mark)
-      ]], default_target_bufnr)
-      jump(default_target_bufnr, location(default_target_uri, 0, 9, 0, 9))
 
-      local mark = exec_lua([[return vim.inspect(vim.api.nvim_buf_get_mark(..., "'"))]], default_target_bufnr)
+      funcs.nvim_win_set_cursor(0, {2, 3})
-      eq('{ 2, 0 }', mark)
+      jump(location(0, 9, 0, 9))
-    end)
 
-    it('should not push item to tagstack if destination is the same as source', function()
+      mark = funcs.nvim_buf_get_mark(target_bufnr, "'")
-      -- Set cursor at the 2nd line, 1st character. This is the source position
+      eq({ 2, 3 }, mark)
-      -- for the test, and will also be the destination one, making the cursor
-      -- "motionless", thus not triggering a push to the tagstack.
-      exec_lua(string.format([[
-        vim.api.nvim_win_set_buf(0, %d)
-        vim.api.nvim_win_set_cursor(0, {2, 0})
-      ]], default_target_bufnr))
-
-      -- Jump to 'f' in 'foobar', at the 2nd line.
-      jump(default_target_bufnr, location(default_target_uri, 1, 0, 1, 0))
-
-      local stack = exec_lua[[return vim.fn.gettagstack()]]
-      eq(0, stack.length)
-    end)
-
-    it('should not push the same item from same buffer twice to tagstack', function()
-      -- Set cursor at the 2nd line, 5th character.
-      exec_lua(string.format([[
-        vim.api.nvim_win_set_buf(0, %d)
-        vim.api.nvim_win_set_cursor(0, {2, 4})
-      ]], default_target_bufnr))
-
-      local stack
-
-      -- Jump to 1st line, 1st column.
-      jump(default_target_bufnr, location(default_target_uri, 0, 0, 0, 0))
-
-      stack = exec_lua[[return vim.fn.gettagstack()]]
-      eq({default_target_bufnr, 2, 5, 0}, stack.items[1].from)
-
-      -- Go back to 5th character at 2nd line, which is currently at the top of
-      -- the tagstack.
-      exec_lua(string.format([[
-        vim.api.nvim_win_set_cursor(0, {2, 4})
-      ]], default_target_bufnr))
-
-      -- Jump again to 1st line, 1st column. Since we're jumping from the same
-      -- position we have just jumped from, this jump shouldn't be pushed to
-      -- the tagstack.
-      jump(default_target_bufnr, location(default_target_uri, 0, 0, 0, 0))
-
-      stack = exec_lua[[return vim.fn.gettagstack()]]
-      eq({default_target_bufnr, 2, 5, 0}, stack.items[1].from)
-      eq(1, stack.length)
-    end)
-
-    it('should not push the same item from another buffer twice to tagstack', function()
-      local target_uri = 'file://foo/bar'
-      local target_bufnr = create_buf(target_uri, {'this is a line', 'foobar'})
-
-      -- Set cursor at the 1st line, 3rd character of the default test buffer.
-      exec_lua(string.format([[
-        vim.api.nvim_win_set_buf(0, %d)
-        vim.api.nvim_win_set_cursor(0, {1, 2})
-      ]], default_target_bufnr))
-
-      local stack
-
-      -- Jump to 1st line, 1st column of a different buffer from the source
-      -- position.
-      jump(target_bufnr, location(target_uri, 0, 0, 0, 0))
-
-      stack = exec_lua[[return vim.fn.gettagstack()]]
-      eq({default_target_bufnr, 1, 3, 0}, stack.items[1].from)
-
-      -- Go back to 3rd character at 1st line of the default test buffer, which
-      -- is currently at the top of the tagstack.
-      exec_lua(string.format([[
-        vim.api.nvim_win_set_buf(0, %d)
-        vim.api.nvim_win_set_cursor(0, {1, 2})
-      ]], default_target_bufnr))
-
-      -- Jump again to 1st line, 1st column of the different buffer. Since
-      -- we're jumping from the same position we have just jumped from, this
-      -- jump shouldn't be pushed to the tagstack.
-      jump(target_bufnr, location(target_uri, 0, 0, 0, 0))
-
-      stack = exec_lua[[return vim.fn.gettagstack()]]
-      eq({default_target_bufnr, 1, 3, 0}, stack.items[1].from)
-      eq(1, stack.length)
     end)
   end)