fix(#1785): retain focused node on filter toggle

[Akmadan23]

fixes #1785

I was annoyed by the fact that after clearing the live filter the cursor does not follow the focused node, so I made a simple change that addresses the issue. Maybe it could be enabled or disabled with an option, something like live_filter.focus_node_on_clear. Let me know if I should add it or if it's reasonable to leave it as is.

[gegoune]

Thanks for pr. This has been worked on by @alex-courtis in #1872 but no satisfying solution was found at the time. Would you mind having a look at linked pr to check if concerns raised there are not affecting your solution here?

[Akmadan23]

If file is not opened cursor will jump to first line of tree and remain there when switching filter on and off. Seem like it just doesn't have an entry to jump to.

This is not happening to me, maybe because I'm using utils.focus_file passing the full path of the node, so that if it's visible, it simply focuses that. Or maybe it's because I'm only applying it to the live filter instead of the various filters that Alex applied it to, but I can assure it works fine. Or maybe I completely misunderstood the issue here.

Edit: I misunderstood the issue, I've looked at it for a while and I have some considerations.

Options for "node no longer visible" case:

1. Focus root
2. Focus lowest visible parent folder if present
3. Do nothing

1 is jarring, 3 is current behaviour.

Although 1 can be somewhat jarring, I think it's definitely better than 3.

Things get complicated when it comes to folders and nothing felt quite right.

That seems to be the case, but I'm experimenting a bit and I feel confident that I can find an elegant and working solution, I'll leave a new comment if I get to create something good.

[Akmadan23]

I might have found a solution, I'll paste it here in the comment so that we can discuss it and then I will commit to the PR if it will be approved.

local function reload()
  local node = lib.get_node_at_cursor()
  local visible_nodes = require("nvim-tree.core").get_explorer().nodes
  reloaders.reload_explorer()

  while node do
    local found_node, _ = utils.find_node(visible_nodes, function(n)
      return n.absolute_path == node.absolute_path
    end)

    if found_node or node.parent == nil then
      utils.focus_file(node.absolute_path)
      break
    end

    node = node.parent
  end
end

This reload function is applied exactly like @alex-courtis did in #1872, and according to my tests it fixes all the issues presented above. Let me know if it actually makes sense or if I'm missing something important here.

By the way, all this checks are completely useless applied the live filter, because when it gets cleared nodes cannot disappear but only reappear, so if a node has focus it will keep it with no edge cases. That's why at first I thought it was immune to that problem.

[alex-courtis]

I might have found a solution, I'll paste it here in the comment so that we can discuss it and then I will commit to the PR if it will be approved.

I'm excited!

Please push or create a new branch and I'll have a play with it.

[alex-courtis]

Test cases:

• live filter
• B
• C
• H
• I
• U
• focus parent - child not selected
• focus closed parent
• child node selected
• group_empty

[alex-courtis]

This feels good to me, and it's how I would build the tree from scratch today.

1 "focus root" is different to current behaviour
3 "do nothing" is current behaviour but it is odd

I'm happy either way. Your call @gegoune !

[Akmadan23]

About the requested changes, is it better to add another commit or do you prefer a force push with a single commit (in my opinion much clearer)?

[gegoune]

Please push individual commits, we can squash on merge.

@alex-courtis either way, really, if it works properly and consistently.

[gegoune]

I gave it a very quick test - doesn't seem to work very well for me. After clearing filters it is parent directory that gets focus quite often. Clearing live filter lives cursor at the position where live filtering input was.

[Akmadan23]

After clearing filters it is parent directory that gets focus quite often.

I'm not sure what you mean here...

Clearing live filter lives cursor at the position where live filtering input was.

Yeah, I didn't consider that scenario, the easiest solution is to focus root, but we could also save the last focused node in a local variable and then fall back to that if no node is found under the cursor on clear.

Moreover, since also collapse_all has this issue, I think would be handy to move the reload function to the utils module, something like utils.focus_node_or_parent(node), so that it can be easily reused without rewriting it. Wouldn't it be nice?

[alex-courtis]

Yeah, I didn't consider that scenario, the easiest solution is to focus root, but we could also save the last focused node in a local variable and then fall back to that if no node is found under the cursor on clear.

That would feel nice, quite similar to current behaviour, but actually correct in both cases.

I'd be grateful if you could attempt that @Akmadan23

[alex-courtis]

Moreover, since also collapse_all has this issue, I think would be handy to move the reload function to the utils module, something like utils.focus_node_or_parent(node), so that it can be easily reused without rewriting it. Wouldn't it be nice?

That sounds good. Can we please do that refactor in a separate PR after this?

[Akmadan23]

Can we please do that refactor in a separate PR after this?

Sure, for now I'm pushing the fix for the live filter issue which is actually really simple.

[alex-courtis]

This is fantastic!

I'll dogfood this one for a week and get a feel for it.

[alex-courtis]

This is looking really good... we are close.

[alex-courtis]

This applies only to live filter, not the other toggle filters.

Do we need to apply it there as well?

[Akmadan23]

I don't think it would be useful because when toggling any other filter there will always be a focused node, unless the really rare case when a filter gets toggled when the live filter is already applied and the prompt is focused.

[alex-courtis]

This will be a future problem when we finally sort out multiple trees / tabs.

Can you please store this at view.View?

[Akmadan23]

Sure, that makes total sense.

[alex-courtis]

Dogfooding for a week.

[alex-courtis]

Many thanks for your contribution.

[Akmadan23]

It was a pleasure.