fix(renderer): clean out edge cases of cursor.position.restore (#1355)

Author: pysan3

lua/neo-tree/sources/filesystem/init.lua

@@ -85,7 +85,6 @@ local follow_internal = function(callback, force_show, async)
     end
   end
 
-  state.position.is.restorable = false -- we will handle setting cursor position here
   fs_scan.get_items(state, nil, path_to_reveal, function()
     show_only_explicitly_opened()
     renderer.focus_node(state, path_to_reveal, true)
@@ -129,12 +128,7 @@ M._navigate_internal = function(state, path, path_to_reveal, callback, async)
 
   if path_to_reveal then
     renderer.position.set(state, path_to_reveal)
-    log.debug(
-      "navigate_internal: in path_to_reveal, state.position is ",
-      state.position.node_id,
-      ", restorable = ",
-      state.position.is.restorable
-    )
+    log.debug("navigate_internal: in path_to_reveal, state.position=", state.position.node_id)
     fs_scan.get_items(state, nil, path_to_reveal, callback)
   else
     local is_current = state.current_position == "current"

lua/neo-tree/sources/manager.lua

@@ -39,9 +39,7 @@ local function create_state(tabid, sd, winid)
   state.tabid = tabid
   state.id = winid or tabid
   state.dirty = true
-  state.position = {
-    is = { restorable = false },
-  }
+  state.position = {}
   state.git_base = "HEAD"
   events.fire_event(events.STATE_CREATED, state)
   table.insert(all_states, state)
@@ -527,11 +525,6 @@ M.reveal_current_file = function(source_name, callback, force_cwd)
   local state = M.get_state(source_name)
   state.current_position = nil
 
-  -- When events trigger that try to restore the position of the cursor in the tree window,
-  -- we want them to ignore this "iteration" as the user is trying to explicitly focus a
-  -- (potentially) different position/node
-  state.position.is.restorable = false
-
   local path = M.get_path_to_reveal()
   if not path then
     M.focus(source_name)

lua/neo-tree/ui/renderer.lua

@@ -522,8 +522,10 @@ M.focus_node = function(state, id, do_not_focus_window, relative_movement, botto
     end
     local success, err = pcall(vim.api.nvim_win_set_cursor, state.winid, { linenr, col })
 
-    -- now ensure that the window is scrolled correctly
     if success then
+      -- forget about cursor position as it is overwritten
+      M.position.clear(state)
+      -- now ensure that the window is scrolled correctly
       local execute_win_command = function(cmd)
         if vim.api.nvim_get_current_win() == state.winid then
           vim.cmd(cmd)
@@ -647,59 +649,51 @@ M.position = {
       log.debug("There's already a position saved to be restored. Cannot save another.")
       return
     end
-
     if state.tree and M.window_exists(state) then
       local win_state = vim.api.nvim_win_call(state.winid, vim.fn.winsaveview)
       state.position.topline = win_state.topline
       state.position.lnum = win_state.lnum
       log.debug("Saved cursor position with lnum: " .. state.position.lnum)
       log.debug("Saved window position with topline: " .. state.position.topline)
-      -- Only need to restore the cursor state once per save, comes
-      -- into play when some actions fire multiple times per "iteration"
-      -- within the scope of where we need to perform the restore operation
-      state.position.is.restorable = true
     end
   end,
   set = function(state, node_id)
     if not type(node_id) == "string" and node_id > "" then
       return
     end
     state.position.node_id = node_id
-    state.position.is.restorable = true
+  end,
+  clear = function (state)
+    log.debug("Forget about cursor position.")
+    -- Clear saved position, so that we can save another position later.
+    state.position.topline = nil
+    state.position.lnum = nil
+    -- After focusing a node, we clear it so that subsequent renderer.position.restore don't
+    -- focus on it anymore
+    state.position.node_id = nil
   end,
   restore = function(state)
-    if state.position.is.restorable then
-      if state.position.topline and state.position.lnum then
-        log.debug("Restoring window position to topline: " .. state.position.topline)
-        log.debug("Restoring cursor position to lnum: " .. state.position.lnum)
-        vim.api.nvim_win_call(state.winid, function()
-          vim.fn.winrestview({ topline = state.position.topline, lnum = state.position.lnum })
-        end)
-        -- Clear saved position, so that we can save another position later.
-        state.position.topline = nil
-        state.position.lnum = nil
-      end
-      if state.position.node_id then
-        log.debug("Focusing on node_id: " .. state.position.node_id)
-        M.focus_node(state, state.position.node_id, true)
-        -- After focusing a node, we clear it so that subsequent renderer.position.restore don't
-        -- focus on it anymore
-        state.position.node_id = nil
-      end
-    else
-      log.debug("Position is not restorable")
+    if state.position.topline and state.position.lnum then
+      log.debug("Restoring window position to topline: " .. state.position.topline)
+      log.debug("Restoring cursor position to lnum: " .. state.position.lnum)
+      vim.api.nvim_win_call(state.winid, function()
+        vim.fn.winrestview({ topline = state.position.topline, lnum = state.position.lnum })
+      end)
+    end
+    if state.position.node_id then
+      log.debug("Focusing on node_id: " .. state.position.node_id)
+      M.focus_node(state, state.position.node_id, true)
     end
-    state.position.is.restorable = false
+    M.position.clear(state)
   end,
-  is = { restorable = true },
 }
 
 ---Redraw the tree without relaoding from the source.
 ---@param state table State of the tree.
 M.redraw = function(state)
   if state.tree and M.tree_is_visible(state) then
     log.trace("Redrawing tree", state.name, state.id)
-    -- every now and then this will fail because the window was closed in 
+    -- every now and then this will fail because the window was closed in
     -- betweeen the start of an async refresh and the redraw call.
     -- This is not a problem, so we just ignore the error.
     local success = pcall(render_tree, state)
@@ -938,7 +932,7 @@ local get_buffer = function(bufname, state)
     vim.api.nvim_buf_set_option(bufnr, "filetype", "neo-tree")
     vim.api.nvim_buf_set_option(bufnr, "modifiable", false)
     vim.api.nvim_buf_set_option(bufnr, "undolevels", -1)
-    autocmd.buf.define(bufnr, "WinLeave", function()
+    autocmd.buf.define(bufnr, "BufDelete", function()
       M.position.save(state)
     end)
   end
@@ -1045,15 +1039,9 @@ M.acquire_window = function(state)
     vim.api.nvim_buf_set_name(state.bufnr, bufname)
     vim.api.nvim_set_current_win(state.winid)
     -- Used to track the position of the cursor within the tree as it gains and loses focus
-    --
-    -- Note `WinEnter` is often too early to restore the cursor position so we do not set
-    -- that up here, and instead trigger those events manually after drawing the tree (not
-    -- to mention that it would be too late to register `WinEnter` here for the first
-    -- iteration of that event on the tree window)
-    win:on({ "WinLeave" }, function()
+    win:on({ "BufDelete" }, function()
       M.position.save(state)
     end)
-
     win:on({ "BufDelete" }, function()
       win:unmount()
     end, { once = true })