Skip to content

fix(#1815): don't schedule find file calls #1820

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

Merged
merged 6 commits into from
Dec 16, 2022

Conversation

alex-courtis
Copy link
Member

@alex-courtis alex-courtis commented Dec 11, 2022

fixes #1815

Optimistic PR for dogfooding.

Both
https://github.com/nvim-tree/nvim-tree.lua/pull/1820/files#diff-d2d7203cea884a09928ac1574693d436d6e3db28e62daabfa53f5f9f7740f51aR157 and

local fname_real = vim.loop.fs_realpath(fname)

protect against any finding of buffers that are not real files i.e. not nvim-tree itself.

@gegoune
Copy link
Collaborator

gegoune commented Dec 11, 2022

Functionally it works, thanks for the fix!

But not exporting find_file() will break for users using it. I am not sure if it ever was meant to be used and given we have an api we should make it the only 'entry' point to nvim-tree (programatically).

Should equivalent be:

      require('nvim-tree').open()
      require('nvim-tree.api').tree.find_file(bufname)

?

@alex-courtis
Copy link
Member Author

But not exporting find_file() will break for users using it. I am not sure if it ever was meant to be used and given we have an api we should make it the only 'entry' point to nvim-tree (programatically).

Ah of course, I wondered why it was public.

There's api.tree.find_file, api.tree.toggle, :NvimTreeFindFile and :NvimTreeFindFileToggle

I'm game to remove it if you are ;)

@alex-courtis
Copy link
Member Author

Quick BufEnter events cause multiple find_file calls, which is a performance hit as it results in an FS and git refresh.

Debounced all events with single context "BufEnter:find_file".

@gegoune
Copy link
Collaborator

gegoune commented Dec 12, 2022

Absolutely, remove it. Even if it was not intended to be used I think that removal warrants notifying users? It will be breaking change for sure.

@alex-courtis
Copy link
Member Author

Even if it was not intended to be used I think that removal warrants notifying users?

a284061

@alex-courtis alex-courtis merged commit 623cecb into master Dec 16, 2022
@alex-courtis alex-courtis deleted the 1815-telescope-update_focused_file-regression branch December 16, 2022 02:01
alex-courtis added a commit that referenced this pull request Dec 16, 2022
…ocused_file with 15ms default (#1820)"

This reverts commit 623cecb.
alex-courtis added a commit that referenced this pull request Dec 16, 2022
…update_focused_file with 15ms default (#1820)""

This reverts commit a8d26bb.
alex-courtis added a commit that referenced this pull request Dec 16, 2022
…ile with 15ms default (#1828)

* Revert "Revert "fix(#1815): don't schedule find_file calls, debounce update_focused_file with 15ms default (#1820)""

This reverts commit a8d26bb.

* fix(#1723): find_file for externally created new file results in folder unable to be opened

* fix(#1723): find_file for externally created new file results in folder unable to be opened
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.

Telescope find_files Fails update_focused_file
2 participants