1836 view debounce delay #1871
1836 view debounce delay #1871
Conversation
| @@ -381,7 +383,9 @@ local function setup_autocommands(opts) | |||
| (filters.config.filter_no_buffer or renderer.config.highlight_opened_files ~= "none") | |||
| and vim.bo[data.buf].buftype == "" | |||
| then | |||
| reloaders.reload_explorer(nil, data.buf) | |||
| utils.debounce("Buf:filter_buffer", opts.view.debounce_delay, function() | |||
There was a problem hiding this comment.
Was the bug caused by having separate categories?
There was a problem hiding this comment.
It's simply because view.debounce_delay was used instead of opts.view.debounce_delay, which resulted in nil exception somewhere else.
| @@ -481,8 +485,10 @@ local function setup_autocommands(opts) | |||
| if opts.modified.enable then | |||
| create_nvim_tree_autocmd({ "BufModifiedSet", "BufWritePost" }, { | |||
| callback = function() | |||
| modified.reload() | |||
| reloaders.reload_explorer() | |||
| utils.debounce("Buf:modified", opts.view.debounce_delay, function() | |||
There was a problem hiding this comment.
This is looking good.
I'd eventually like to have all reload_explorer calls debounced (from within) however that will require some refactoring first. This gradual approach will work well.
Idea: could we use one category for this and "Buf:filter_buffer"? We would need to move modified.reload into reload_explorer, which should be OK as it is not expensive. The category could just be "Buf".
"BufEnter:find_file" will need to remain separate for now due to its extra complexity.
There was a problem hiding this comment.
Hmmm, that sounds sensible, though I feel like if we do this too much then reload_explorer will become slower and slower as we add more feature to it.
I actually have another refactoring idea after I finished modified, it goes like this:
We split nvim-tree into three distinct part: core, external modules & line builder.
Git, diagnostics, modified & opened file becomes external modules. They interact with core to attach attributes to paths (i.e. directories & files). Core can propagate attributes to parents / children at the external module's request.
Core calls line builder with all the path's attributes + other info like name, depth. Line builder returns everything the core needs to display including padding, signcolumn, highlights etc.
This would allow us to do a few things:
- Only calling the line builder if any attribute for that specific node has changed.
- Let user add their own external module / override the line builder.
There was a problem hiding this comment.
Yes.
We split nvim-tree into three distinct part: core, external modules & line builder.
I like this approach.
Files and git status are somewhat detached in that they don't trigger a full refresh, only a draw for the affected directory.
Everything else does a full refresh, which is unnecessary and expensive. Examples: filter, most nvim events, fs operations.
They interact with core to attach attributes to paths (i.e. directories & files). Core can propagate attributes to parents / children at the external module's request.
That would work very nicely: core can decide upon the least expensive action and propagate that.
It would be fantastic if you could start on this one: we could "proof of concept" this by having some events call reload.refresh_nodes_for_path or just reload.refresh_node.
This can be done iteratively ande we can split up the functionality as we go.
There was a problem hiding this comment.
I just would like to add that we should not be afraid of breaking changes if benefits out weight inconveniences of one off config changes for users.
Project longevity and maintainability is more important.
There was a problem hiding this comment.
if benefits out weight inconveniences of one off config changes for users.
Absolutely. We have successfully made a few such small changes with no reported issues.
There was a problem hiding this comment.
api.tree.toggle_no_buffer_filter()
api.tree.expand_all()doesn't work as you'd expect, as the filter is debounced and completes after expansion.
Having the toggle draw synchronously with no refresh would resolve that.
There was a problem hiding this comment.
It would be fantastic if you could start on this one: we could "proof of concept" this by having some events call
reload.refresh_nodes_for_pathor justreload.refresh_node.This can be done iteratively ande we can split up the functionality as we go.
Sounds great, I'll think about opening a tracking issue and start with highilght_opened_files in a few days.
Do we have any plan for versioning nvim-tree? I think it'd be nice if we put those refactoring onto release candidate for nvim-tree 2.0.
There was a problem hiding this comment.
Big 👍
Versioning is a work in progress. We don't have any immediate plans to break anything large. Also... this change would be non-breaking.
| @@ -381,7 +383,9 @@ local function setup_autocommands(opts) | |||
| (filters.config.filter_no_buffer or renderer.config.highlight_opened_files ~= "none") | |||
| and vim.bo[data.buf].buftype == "" | |||
| then | |||
| reloaders.reload_explorer(nil, data.buf) | |||
| utils.debounce("Buf:filter_buffer", opts.view.debounce_delay, function() | |||
|
Many thanks for all your efforts @chomosuke , nvim-tree is always improving! |
closes #1836.
This PR also includes #1855 (& fixes the bugs it contains :) ).