Add action to open a subtree recursively

[LunarWatcher]

Opening entire subtrees is incredibly nice to have. Seeing as this hasn't been implemented due to the difficulty of implementation, I decided to take a stab at it.

This fixes at least #39, #320, and #412.

---

The current state of the PR:

• in and stay both work (in a bare minimum proof-of-concept-type way; take that with a truckload of salt)
• <Plug> maps have been added and documented (or at least their existence is mentioned in the docs; only did half a glance at the docs, not sure if they need to be mentioned elsewhere)
• There's a single test pair in the node.vimspec file. I assume more tests would be nice, but I haven't looked extensively, so not sure where they'd go but I'm not seeing any equivalent tests that I can copy-pasta and modify. I can still add some if desired

Currently, the PR is a draft, because there are a couple problems that I can't quite deal with on my own:

1. Using s:Promise.all to merge the subtree promises feels like a hack, but I'm not seeing any obviously better options in vital.vim for dealing with arrays of promises. There's also plenty of discarded values, in part because a:nodes appears to be mutated, so just directly returning it is convenient. I'm not sure if this means the entire tree is refreshed, or if it's just some convenient abstraction representing change.
   • If the return values have to be used (rather than relying on a:nodes' mutability), the implementation complexity goes up considerably, particularly when it's time to merge the return values. I can store and return ns (whatever that is anyway) from expand, but the difficulty lies in merging a list of other nses into the current ns. A very, very quick and shallow search only found a method that adds a single node to a:nodes, and not one that trivially merges two a:nodes lists together. I could make one, but I opted not to when a:nodes is mutable, and there might be an even better approach that makes the entire thing pointless
2. There aren't any default keys set for the new actions. This is largely because I don't know what standards make sense, as I use my own keys in my config that more closely align with NERDTree than fern. Might also be an idea to do that separately from this PR

There's probably more stuff to grill in a code review as well (including bugs to be found; I do suspect using a:nodes instead of ns return values is something that'll come back to haunt me, but I haven't found anything obvious yet). A chunk of the implementation was based on existing functions (particularly the convenient s:expand_recursively that I shamelessly copy-pasta'd for a starting point), so there's probably a fair share of copy-pasta errors and redundancies as well

[LunarWatcher]

I'm not going to pretend to understand why, but for some reason, async.get_child_nodes makes the first layer of subdirectories (i.e. the first level of folders inside the recursively expanded folder) be registered as closed, while still being open visually. Hacked around that with the trivial assumption that if the expanded tree isn't empty, there has to be more nodes after expanding.

Aside that, I've been testing it since I made the initial implementation, and no problems so far. There's still probably more to do, but it's arguably good enough for now; the PR is as ready as I can get it

[lambdalisue]

Thanks for your contribution. It seems OK so could you re-organize your commits (to remove some draft commits or so on) or shall I squash all commits into a single one?

[LunarWatcher]

Squashing is easier and faster

[brandon1024]

Interesting changes! A few questions/comments.

At first glance, it was a bit unclear what these changes do. For clarity in the commit messages and fern documentation, I think it would be good to mention recursive expansion of trees. For example:

*<Plug>(fern-action-expand-tree:stay)*
	Recursively expand the tree on a cursor node and keep the cursor on the root node.

For directory structures that are quite deep or have significant breadth, expanding a tree recursively can be a fairly significant operation. It would be nice if the user would be notified that the request is being fulfilled. Maybe with a simple message, or some other mechanism.

I checked out your changes locally and it looks like the recursive tree expansion doesn't work when the node is already in the expanded state. To reproduce:

• expand a directory <Plug>(fern-action-expand-tree)
• collapse a subdirectory
• move cursor back to the parent directory
• try expanding again <Plug>(fern-action-expand-tree)

Other than that, changes look good so far. Looking forward to seeing this integrated :-)

[lambdalisue]

At first glance, it was a bit unclear what these changes do. For clarity in the commit messages and fern documentation, I think it would be good to mention recursive expansion of trees. For example:

Sounds good. Please mention that in docs @LunarWatcher

I checked out your changes locally and it looks like the recursive tree expansion doesn't work when the node is already in the expanded state. To reproduce:

I've noticed that too, but I feel like that's a limitation and it's easy to start over so we can merge.

[LunarWatcher]

At first glance, it was a bit unclear what these changes do. For clarity in the commit messages and fern documentation, I think it would be good to mention recursive expansion of trees. For example:

Sounds good, will do.

For directory structures that are quite deep or have significant breadth, expanding a tree recursively can be a fairly significant operation. It would be nice if the user would be notified that the request is being fulfilled. Maybe with a simple message, or some other mechanism.

There's already an update spinner system, but it doesn't seem to stay on the node being recursively expanded. Would be optimal to integrate with it, but not sure if that's an option

I checked out your changes locally and it looks like the recursive tree expansion doesn't work when the node is already in the expanded state. To reproduce:

That's currently half intentional, based on the existing function for async_expand_node just refreshing instead. Dropping it causes a duplication bug that I think I can fix by just conditionally calling #expand(). Will look into it now

[LunarWatcher]

It was not that easy. Got this far:

  return s:Promise.new({ resolve -> 
        \   s:Lambda.if(
        \     a:node.status isnot# s:STATUS_EXPANDED,
        \     { -> resolve(
        \       fern#internal#node#expand(a:node, a:nodes, a:provider, a:comparator, a:token)
        \     )},
        \     { -> resolve(a:nodes) }
        \  )})
        \.then({ ns -> s:set_expanded(a:node.__key, ns) })
        \.then({ ns -> s:Lambda.let(state, 'ns', ns) })
        \.then({ -> fern#internal#node#children(a:node, a:provider, a:token) })
        \.then(s:AsyncLambda.filter_f({ child -> child isnot# v:null && child.status isnot# s:STATUS_NONE }))
        \.then(s:AsyncLambda.map_f({child_node -> fern#internal#node#expand_tree(child_node, state.ns, a:provider, a:comparator, a:token) }))
        \.then({ subtree_promises -> s:Promise.all(subtree_promises) })
        \.then({ -> l:state.ns })
        \.finally({ -> Done() })

with

function! s:set_expanded(key, nodes) abort
  let index = fern#internal#node#index(a:key, a:nodes)
  if index is# -1
    return a:nodes
  endif
  let a:nodes[index].status = s:STATUS_EXPANDED
  return a:nodes
endfunction

As a probably misguided guess at the problem.

For some reason, status isn't set to STATUS_EXPANDED, which prevents the expand guard from being effective. The expand guard isn't even necessary if the status was set properly, as the expand function has built-in guards. This also doesn't make sense seeing as expanding normally and then recursively expanding results in the exact same behaviour (i.e. manually expand all using the default expand action, then recursively expand the root), at least in my very limited test runs. There might be a bigger bug here, but everything else works fine, so I'm not sure what causes it. Would strongly recommend looking at this separately later.

Anyway, the last commit includes the requested doc wording change, and I noticed what made the spinners work, so I added one of those too, so that's all good.

[brandon1024]

Anyway, the last commit includes the requested doc wording change, and I noticed what made the spinners work, so I added one of those too, so that's all good.

Documentation changes look good, thanks! 👌 I'll go through the rest of the changes when I'm not at work 😆

There's already an update spinner system, but it doesn't seem to stay on the node being recursively expanded. Would be optimal to integrate with it, but not sure if that's an option

That would be nice if we could make that work! If not, no big deal, that could be a later enhancement.

Answering earlier comments:

I'm not going to pretend to understand why, but for some reason, async.get_child_nodes makes the first layer of subdirectories (i.e. the first level of folders inside the recursively expanded folder) be registered as closed, while still being open visually.

Weeeird. This sounds like defective behaviour to me; shouldn't get_child_notes be completely idempotent? I took a brief glance to see if I can spot where this is happening, no luck. It's proabably somewhere in the dense fern#internal#node#children(). @lambdalisue any ideas? I'd need to sit down and really go through this in depth. It would be fun to find and correct this behaviour and see if it breaks anything.

[LunarWatcher]

That would be nice if we could make that work! If not, no big deal, that could be a later enhancement.

That's the bit I got working

Weeeird. This sounds like defective behaviour to me;

I do wonder if that function also was fine, but that it's weird behaviour caused by state not being properly set. Can't be arsed to check though, but it might be the same problem. The only difference is that the bug I failed to fix to recursively open already open folders can be triggered an unlimited number of times. /shrug though, as long as it works, etc

[lambdalisue]

Weeeird. This sounds like defective behaviour to me; shouldn't get_child_notes be completely idempotent? I took a brief glance to see if I can spot where this is happening, no luck. It's proabably somewhere in the dense fern#internal#node#children(). @lambdalisue any ideas? I'd need to sit down and really go through this in depth. It would be fun to find and correct this behaviour and see if it breaks anything.

Actually, I think it's intentional. I'll describe the detail later but the function try to return fresh child nodes that should not be modified (expanded is modified status). So it actually is idempotent.

[lambdalisue]

I noticed that expand is not immutable so I'll fix that first. I guess that makes this PR code clear.

[lambdalisue]

@LunarWatcher @brandon1024

The current implementation relies on the behavior of fern#internal#node#expand rewriting nodes, which no longer works with the previous fix. Therefore, I have created a separate PR with the core part rewritten, so could you please try that one?

New one works OK with already expanded nodes.

[LunarWatcher]

The current implementation relies on the behavior of fern#internal#node#expand rewriting nodes, which no longer works with the previous fix. Therefore, I have created a separate PR with the core part rewritten, so could you please try that one?

Seems to work. Feels a bit slower, but nothing too noticeable. I'll close this PR in favour of the other one