Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

window is scrolled up after collapsing node #996

Closed
MunifTanjim opened this issue Jun 17, 2023 · 30 comments · Fixed by #1274 or #1377
Closed

window is scrolled up after collapsing node #996

MunifTanjim opened this issue Jun 17, 2023 · 30 comments · Fixed by #1274 or #1377
Labels
bug Something isn't working
Milestone

Comments

@MunifTanjim
Copy link
Contributor

I was trying neo-tree on https://github.com/qmk/qmk_firmware for checking performance measurement.

When a lots of folders are expanded in keyboards folder, and I collapse one of them, the window is scrolled upwards and so the collapsed node jumps at the bottom.

Kapture 2023-06-18 at 01 02 37

I debugged a bit, and these are the code responsible for the unnecessary jump in this case:

if expected_bottom_line > win_height then
execute_win_command("normal! zb")
local top = vim.fn.line("w0", state.winid)
local bottom = vim.fn.line("w$", state.winid)
local offset_top = top + (expected_bottom_line - bottom)
execute_win_command("normal! " .. offset_top .. "zt")
pcall(vim.api.nvim_win_set_cursor, state.winid, { linenr, col })
elseif win_height > linenr then
execute_win_command("normal! zb")
elseif linenr < (win_height / 2) then
execute_win_command("normal! zz")
end

@miversen33
Copy link
Collaborator

@MunifTanjim #953 is supposed to address this. Can you see if you can recreate this while using the main branch?

@miversen33 miversen33 added the question Further information is requested label Jul 3, 2023
@MunifTanjim
Copy link
Contributor Author

Can you see if you can recreate this while using the main branch?

Yes, this still happens.

@jemag
Copy link

jemag commented Nov 16, 2023

I can reproduce the issue:

2023-11-15.21-11-50.mp4

This makes it a bit painful to use on large mono repos with lots of deeply nested folders.

@miversen33 since the question was answered by @MunifTanjim, perhaps it would be better to remove the question label and add the bug one.

@pysan3 pysan3 added bug Something isn't working and removed question Further information is requested labels Nov 16, 2023
@georgeguimaraes
Copy link
Contributor

When you open files, does the neotree window jump to the top too? This is happening to me in a clean config:

CleanShot.2023-11-17.at.16.06.22.mp4

(from #1025)

@miversen33
Copy link
Collaborator

Eww. Ya that shouldn't be happening. I haven't had basically any time to dig into any of the current bugs that interest me in Neo-tree. But this one seems to exist still (and may not have been completely worked out before). I do not commit to fixing it, but I will be spending some time this week off work poking it. I need a break from some existing projects I have been working on anyway lol

@georgeguimaraes
Copy link
Contributor

I think I found a workaround for my issue (which is the neo-tree window jumping to the top when opening a file). Please see my comment here #1025 (comment)

@miversen33
Copy link
Collaborator

Just popping back in to say that I can recreate this issue with the following configuration

-- Minimal configuration
-- mini.lua
-- Use with the --clean -u flags. EG nvim --clean -u mini.lua
-- This config will create a temp directory and will blow away that temp directory
-- everytime this configuration is loaded. Great for simulating a new installation
-- of a plugin

-- Setting some basic vim options
-- Some junk because I am sick of formatting tables in print
local _print = _G.print
local clean_string = function(...)
    local args = { n = select("#", ...), ... }
    local formatted_args = {}
    for i = 1, args.n do
        local item = select(i, ...)
        if not item then item = 'nil' end
        local t_type = type(item)
        if t_type == 'table' or t_type == 'function' or t_type == 'userdata' then
            item = vim.inspect(item)
        end
        table.insert(formatted_args, item)
    end
    return table.concat(formatted_args, ' ')
end
_G.print = function(...)
    _print(clean_string(...))
end

vim.opt.mouse = 'a'
vim.opt.termguicolors = true
-- If you want to play around with this, you can set the do_clean
-- variable to false. This will allow changes made to
-- underlying plugins to persist between sessions, while
-- still keeping everything in its own directory so
-- as to not affect your existing neovim installation.
--
-- Setting this to true will result in a fresh clone of
-- all modules
local do_clean = false
local sep = vim.loop.os_uname().sysname:lower():match('windows') and '\\' or
'/'                                                                              -- \ for windows, mac and linux both use \
local root = vim.fn.fnamemodify("./.repro", ":p")
if vim.loop.fs_stat(root) and do_clean then
    print("Found previous clean test setup. Cleaning it out")
    -- Clearing out the mods directory and recreating it so
    -- you have a fresh run everytime
    vim.fn.delete(root, 'rf')
end

-- DO NOT change the paths and don't remove the colorscheme

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
    vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
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.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
    "folke/tokyonight.nvim",
    {
        "nvim-neo-tree/neo-tree.nvim", -- File Explorer
        branch = "main",
        dependencies = {
            "nvim-lua/plenary.nvim",
            "kyazdani42/nvim-web-devicons", -- not strictly required, but recommended
            "MunifTanjim/nui.nvim",
            "onsails/lspkind-nvim", -- Document Symbols
        },
        cmd = { "Neotree" },
        keys = {
            { "<Leader>e", "<Cmd>Neotree<CR>" }, -- change or remove this line if relevant
        },
    }
    -- add any other plugins here
}

require("lazy").setup(plugins, {
    root = root .. "/plugins",
})

vim.opt.splitright = true
vim.opt.splitbelow = true


vim.cmd.colorscheme("tokyonight")
-- add anything else here

Recreated on version 3.13.0. To recreate it, simply use the reproduction config, open a directory that has a decent number of items in it (doesn't need to be more then 1 level, though its easier to reproduce if there is some deep nesting), and open a file. You will notice Neo-tree dive back to the top of the window after focus leaves. If you are not able to recreate it, ensure you have scrolled down the neo-tree window before opening a file. It only seems to happen if the top node of the tree is not in rendered when focus leaves the window. Shrinking your terminal window vertically can make it easier to recreate as well. I may throw together a dockerfile to easily recreate this but in theory anyone should be able to.

With all that said, I have what I need to recreate the issue, I just need to find some motivation to poke the problem. Its almost certainly related to the redraw mechanism but beyond that, I am not sure.

@georgeguimaraes
Copy link
Contributor

@miversen33 i thought that my issue and this one were related, but I’ve changed my mind.

So, the issue you’re describing is being tracked at #1025. I’ve debugged a lot there. Can you take a look?

@miversen33
Copy link
Collaborator

@miversen33 i thought that my issue and this one were related, but I’ve changed my mind.

So, the issue you’re describing is being tracked at #1025. I’ve debugged a lot there. Can you take a look?

I read that as well. I am not convinced they aren't related. For now I will track this issue here as opposed to spreading it between various issues.

@miversen33
Copy link
Collaborator

I have a suspicion this issue is actually related to nui.nvim. I have dug pretty deep into the redraw/draw code of Neo-tree if you remove basically all logic around how and when a redraw is called, this issue is still recreatable. And redraw itself is actually a nui call, Neo-tree itself is not generating the tree being displayed, it is asking Nui to regenerate the tree it created before. Not that this gives any real clarity on the issue, just shifts my focus a bit.

Going deeper 🙃

@cseickel
Copy link
Contributor

@miversen33 I would look at these places if you haven't:

M.focus_node = function(state, id, do_not_focus_window, relative_movement, bottom_scroll_padding)

Both involve restoring the window position and focused node after a refresh. If it's Neo-tree code that is causing it, it's probably because these are doing the wrong thing or not being called when they should be.

@jemag
Copy link

jemag commented Nov 23, 2023

one thing to mention is that I can partly workaround the scrolling up issue using:

    {
      event = "before_render",
      handler = function()
        savedview = vim.fn.winsaveview()
      end,
    },
    {
      event = "after_render",
      handler = function()
        if savedview ~= nil then
          vim.fn.winrestview(savedview)
        end
      end,
    },

although it does seem to cause some problems in other situations (e..g. when using the reveal feature).

Since the code already makes use of winsaveview and winrestview my guess is that they might indeed not be called at the proper times.

@miversen33
Copy link
Collaborator

miversen33 commented Nov 23, 2023

@miversen33 I would look at these places if you haven't:

M.focus_node = function(state, id, do_not_focus_window, relative_movement, bottom_scroll_padding)

Both involve restoring the window position and focused node after a refresh. If it's Neo-tree code that is causing it, it's probably because these are doing the wrong thing or not being called when they should be.

Fun fact, if you simply move your cursor from neotree to an open file, you can replicate this issue. I put breakpoints in basically all the spots you listed here and none of them are being hit during this movement

neotree-borked.mp4

Edit:
Adding log created while replicating both the jumping while opening and jumping while focus change issues

Edit 2:
Apparently that was the entire file since forever, narrowed the log down to just the instance used to recreate the issues above
neo-tree.nvim.log

Edit 3:
Adding more specific logs for

neo-tree-jump-from-tree-to-file.log
neo-tree-open-file.log
neo-tree-shifting-on-directory-open-and-close.log

@miversen33
Copy link
Collaborator

miversen33 commented Nov 23, 2023

@cseickel there is an interesting set of logs in the neo-tree-jump-from-tree-to-file.log file

[TRACE Thu 23 Nov 2023 09:42:58 AM CST] ...miversen/git/neo-tree.nvim/lua/neo-tree/events/queue.lua:81: Handler buffers.before_render for before_render called successfully.
[TRACE Thu 23 Nov 2023 09:42:58 AM CST] .../miversen/git/neo-tree.nvim/lua/neo-tree/ui/renderer.lua:1165: Preserving expanded nodes
[DEBUG Thu 23 Nov 2023 09:42:58 AM CST] .../miversen/git/neo-tree.nvim/lua/neo-tree/ui/renderer.lua:769: Tree already exists and buffer is valid, skipping creation filesystem 1
[DEBUG Thu 23 Nov 2023 09:42:58 AM CST] .../miversen/git/neo-tree.nvim/lua/neo-tree/ui/renderer.lua:757: Setting expanded nodes
[TRACE Thu 23 Nov 2023 09:42:58 AM CST] .../neo-tree.nvim/lua/neo-tree/sources/common/container.lua:309: wanted width: 17 actual width: 37
...
[DEBUG Thu 23 Nov 2023 09:42:58 AM CST] .../miversen/git/neo-tree.nvim/lua/neo-tree/ui/renderer.lua:674: Position is not restorable

This happens 2 more times before eventually we get this line

[TRACE Thu 23 Nov 2023 09:42:58 AM CST] ...miversen/git/neo-tree.nvim/lua/neo-tree/events/queue.lua:97: Firing event: after_render with args: {
_ready = true,
async_directory_scan = "auto",
bind_to_cwd = true,
..
[TRACE Thu 23 Nov 2023 09:42:58 AM CST] ...miversen/git/neo-tree.nvim/lua/neo-tree/events/queue.lua:97: Firing event: after_render with args: {
_ready = true,
async_directory_scan = "auto",
bind_to_cwd = true,
...
[TRACE Thu 23 Nov 2023 09:42:58 AM CST] ...miversen/git/neo-tree.nvim/lua/neo-tree/events/queue.lua:97: Firing event: git_status_changed with args: {
git_root = "/home/miversen/git/netman.nvim",
git_status = {
["/home"] = "M",
...
[TRACE Thu 23 Nov 2023 09:42:58 AM CST] .../miversen/git/neo-tree.nvim/lua/neo-tree/ui/renderer.lua:685: Redrawing tree filesystem 1
[TRACE Thu 23 Nov 2023 09:42:58 AM CST] .../neo-tree.nvim/lua/neo-tree/sources/common/container.lua:309: wanted width: 17 actual width: 37
...

That reads like what I am seeing (where the tree jumps to the top after focus lost), but I could just be seeing things. Thoughts?

@georgeguimaraes
Copy link
Contributor

georgeguimaraes commented Nov 23, 2023

Fun fact, if you simply move your cursor from neotree to an open file, you can replicate this issue. I put breakpoints in basically all the spots you listed here and none of them are being hit during this movement

In this case, I'd assume that because your file is modified, the handler for the git_status_changed event forces the redraw. In my case, it was because of the vim_buffer_added event.

@georgeguimaraes
Copy link
Contributor

georgeguimaraes commented Nov 23, 2023

If I change the handler to call M.follow(args.afile) after manager.git_status_changed, it kind of works, but M.follow moves the focused node to respect scrolloff, which almost always means not the same as before.

@cseickel
Copy link
Contributor

That reads like what I am seeing (where the tree jumps to the top after focus lost), but I could just be seeing things. Thoughts?

The "position is not restorable" is definitely something I would dig into. I believe that we do need to restore the position or the NuiTree redraw will cause the window cursor position to reset to line 1.

It is normal to have several redraws just because of the async nature of certain events. Sometimes git is much slower than the filesystem scan so we go ahead and show the files first then update git status with a second pass when that comes in. Also diagnostics update all the time so redraws happen then too.

@miversen33
Copy link
Collaborator

I got pretty deep into the weeds on this last week but haven't been able to figure out what exactly is happening (life does what life do). I am however still investigating this

@miversen33
Copy link
Collaborator

miversen33 commented Dec 9, 2023

Still haven't worked out exactly what is going on here but I have found a workaround. The issue is related to the filesystem re-rendering the tree to show the appropriate git-status for items in the tree. The workaround is to disable git status. IE

require("neo-tree").setup({enable_git_status = false})

I don't know exactly why this is causing issues but disabling it completely removes the weird jumpy behavior on my machines

@serranomorante
Copy link

serranomorante commented Dec 9, 2023

enable_git_status alone wasn't enough on my case to fix the weird jumping behavior. I had to disable diagnostics as well.

Then, enable_opened_markers = false will also fix the "jumps" when opening a file issue. #1025

require("neo-tree").setup({
    enable_git_status = false,
    enable_diagnostics = false,
    enable_opened_markers = false,
    enable_modified_markers = false

Now I'm able to use neo-tree normally again 👍

@miversen33
Copy link
Collaborator

miversen33 commented Dec 9, 2023

That tracks. My original theory about this being a possible nui issue is gone, I believe this is a "race" condition where multiple redraws are happening at basically the same time and the "winner" ends up seeing the tree back to the top for some reason.

Those flags are all used by the filesystem source to add various icons and colors to the tree nodes to indicate the state of them (IE, a file has been modified, there's a problem with a file, etc).

I'm unsure the best approach to fix this, and I'm not sure exactly where in the code this issue is happening, but maybe this is enough to stoke some coals rumbling about in @cseickel or @pysan3 brain and they might think of some cool fix lol. Otherwise I'll keep poking and see if I can narrow down exactly how to solve this

@georgeguimaraes
Copy link
Contributor

enable_git_status alone wasn't enough on my case to fix the weird jumping behavior. I had to disable diagnostics as well.

Then, enable_opened_markers = false will also fix the "jumps" when opening a file issue. #1025

require("neo-tree").setup({
    enable_git_status = false,
    enable_diagnostics = false,
    enable_opened_markers = false,

Now I'm able to use neo-tree normally again 👍

Did you try to downgrade to neovim 0.9.4? That fixed the issue for me and I can enable all those options back.

@miversen33
Copy link
Collaborator

enable_git_status alone wasn't enough on my case to fix the weird jumping behavior. I had to disable diagnostics as well.
Then, enable_opened_markers = false will also fix the "jumps" when opening a file issue. #1025

require("neo-tree").setup({
    enable_git_status = false,
    enable_diagnostics = false,
    enable_opened_markers = false,

Now I'm able to use neo-tree normally again 👍

Did you try to downgrade to neovim 0.9.4? That fixed the issue for me and I can enable all those options back.

This is a valid alternative workaround (I have verified the downgrade does also remove the issue)

@cseickel
Copy link
Contributor

cseickel commented Dec 9, 2023

There must be some change in how neovim is processing events or scheduled executions. IF it is a race condition on redraw requests then my solution would be to using the internal event queue to debounce redraw requests using the same concept as the filesystem.follow --> follow_internal methods:

M.follow = function(callback, force_show)
if vim.fn.bufname(0) == "COMMIT_EDITMSG" then
return false
end
if utils.is_floating() then
return false
end
utils.debounce("neo-tree-follow", function()
return follow_internal(callback, force_show)
end, 100, utils.debounce_strategy.CALL_LAST_ONLY)
end

@georgeguimaraes
Copy link
Contributor

I've proposed a fix on #1274. Can you try it?

@teocns
Copy link

teocns commented Jan 20, 2024

I am getting the same behaviour and none of the suggested fixes have helped. To me it happens while opening opening a directory. I believe I started having this issue from neovim 0.95 onwards

@Hubro
Copy link

Hubro commented Mar 9, 2024

I came here to report this exact issue. Whenever I jump to a file in the tree (:Neotree reveal) or I close a directory with <BS>, neo-tree scrolls to place my cursor at the bottom of the screen, just like the default zb keybind. The only thing keeping my cursor from the bottom of the screen is the 'scrolloff' setting.

image

This shows neo-tree right after closing the just directory with <BS>, i.e. close_node. The directory was centered on the screen before this.

I am on commit 459c603 on Neovim version v0.10.0-dev-dbf6be2. This issue should be re-opened.

@pysan3
Copy link
Collaborator

pysan3 commented Mar 9, 2024

@Hubro Yes update to the latest main branch. It's already fixed.

@Hubro
Copy link

Hubro commented Mar 9, 2024

@Hubro Yes update to the latest main branch. It's already fixed.

Ah, my bad. Since this issue was closed in 2023 I assumed the v3 branch was sufficiently up to date 😅

But it seems like this issue was actually just fixed 3 days ago. In any case, I updated and it worked 🎉

@pysan3
Copy link
Collaborator

pysan3 commented Mar 9, 2024

@Hubro #1377 (comment)

@pysan3 pysan3 added this to the v4.0 milestone Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
9 participants