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

fix(renderer): fix jumping to the top when opening files. #1274

Merged

Conversation

georgeguimaraes
Copy link
Contributor

@georgeguimaraes georgeguimaraes commented Dec 27, 2023

When opening files you had to scroll down to find, the neo-tree window jumped to the top (see #1025).

This is because Neovim changed the way nvim_buf_set_lines works in neovim/neovim@0081549 (neovim/neovim#24824) and later in neovim/neovim#24889

This change in Neovim caused some problems in plugins:

To ensure the window stays fixed, we have to use winsaveview and winrestview (wrapped in M.position) inside render_tree.

Fixes #1025
Fixes #996

When opening files that you had to scroll down to find, the neo-tree
window jumped to the top.

This is because Neovim changed the way `nvim_buf_set_lines` works. To
make sure the window stays fixed, we have to use `winsaveview` and
`winrestview`.
@georgeguimaraes
Copy link
Contributor Author

Please @miversen33 can you try it and see if I made a mess?

@georgeguimaraes
Copy link
Contributor Author

I have a fix for #996 that builds on top of this PR. I'll have my commit cleaned up and I'll test a few edge cases and I'll update this PR

@georgeguimaraes georgeguimaraes changed the title fix(renderer): fix jumping to the top when opening files. Fixes #1025 fix(renderer): fix jumping to the top when opening files. Dec 27, 2023
@georgeguimaraes
Copy link
Contributor Author

georgeguimaraes commented Dec 27, 2023

Ok, now 6cf6f37 fixes #996 as well. I've tested some cases with it, but I am not confident of such change, even though it uses well-established functions winsaveview/winrestview.

If you all feel more comfortable, I can remove this commit and we can just focus on #1025.

@miversen33
Copy link
Collaborator

I'll pull this down and review the branch and code this evening, I have a nice long drive ahead of me this afternoon

We can use `lnum` from winsaveview to set our cursor and not depend
on M.follow_focus and state.tree.get_node (which can be nil sometimes).

This way we don't trigger M.follow_focus window centering algorithm when
expanding or closing nodes.
@miversen33
Copy link
Collaborator

I left some notes on this. The overall gist seems good though I will not be the one to sign off on this. I will leave that to either @pysan3 or @cseickel as they are far more active in neo-tree core than myself. They will in theory be able to see potential side effects from this code that I might miss.

lua/neo-tree/ui/renderer.lua Outdated Show resolved Hide resolved
@pysan3
Copy link
Collaborator

pysan3 commented Dec 28, 2023

The change looks awesome on my end! Thanks a lot!

I'll pass it to @cseickel now.

Copy link
Contributor

@cseickel cseickel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me and since @miversen33 and @pysan3 also reviewed I'll skip testing it myself.

Thanks!

@cseickel cseickel merged commit 953313e into nvim-neo-tree:main Dec 29, 2023
1 of 2 checks passed
@georgeguimaraes georgeguimaraes deleted the gg-fix-jumping-to-the-top branch December 29, 2023 03:42
@OliverChao
Copy link

Sorry for mentioning that this pr seems breaking the open action in Neotree reveal.

Open one file and run Neotree reveal, then run <cr> to open the directory of that file.

After running <CR>, it jumps to the file location not toggle the directory.

@cseickel
Copy link
Contributor

Sorry for mentioning that this pr seems breaking the open action in Neotree reveal.

Open one file and run Neotree reveal, then run <cr> to open the directory of that file.

After running <CR>, it jumps to the file location not toggle the directory.

That is true. Beyond that, any action you take will result in the previously revealed file being focused. That includes opening other files. Whatever the last revealed file is will be permanently stuck and refocused after every render.

I will make this a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Neo-tree window "jumps" when opening files window is scrolled up after collapsing node
5 participants