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

Add action to open a subtree recursively (v2) #483

Merged
merged 17 commits into from Aug 26, 2023
Merged

Conversation

lambdalisue
Copy link
Owner

Original PR is #480

I rewrote fern#internal#node#expand_tree because of #482

@brandon1024
Copy link
Contributor

brandon1024 commented Aug 26, 2023

Edit:

This is actually a defect on main. I'll open an issue :-)

Original

Good first revision! Recursive tree expansion works as I'd expect, very nice :-)

I found a small quirk though. I have some configuration that toggles expansion of a node or opens a leaf. This has stopped working correctly with this revision.

	nmap <buffer> <CR> <Plug>(fern-action-toggle-expand-open)

	nmap <buffer><expr> <Plug>(fern-action-toggle-expand-open)
		\ fern#smart#leaf(
		\   "<Plug>(fern-action-open:select)",
		\   "<Plug>(fern-action-expand)",
		\   "<Plug>(fern-action-collapse)")

When I open Fern and expand a tree for the first time (with <CR>), it works as I would expect. After collapsing the node and expanding the tree again, the first child node is in the expanded state but doesn't render as such.

Here's a screen capture to demonstrate this behaviour. Notice the state of ews-eks when I expand it the second time:

Peek 2023-08-26 08-04

doc/fern.txt Outdated
>
nmap <buffer>
\ <Plug>(fern-action-expand-tree)
\ <Plug>(fern-action-expand-tree:stay)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are missing the closing quote block character here (<).

\.then({ ns -> self.update_nodes(ns) })
\.finally({ -> Profile() })
endfunction
let s:async.expand_tree = funcref('s:async_expand_tree')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something that we should document in fern-develop.txt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should. The other async functions are part of the dev documentation IIRC

@brandon1024
Copy link
Contributor

brandon1024 commented Aug 26, 2023

@lambdalisue @LunarWatcher does it make sense to close one of the PRs we have open? Who wants to own the changes for this feature?

Edit: other PR closed 👍

@lambdalisue
Copy link
Owner Author

@brandon1024 @LunarWatcher fixed and force pushed. Could you try and check?

@LunarWatcher
Copy link
Contributor

LGTM

@lambdalisue lambdalisue merged commit e295f0d into main Aug 26, 2023
16 checks passed
@lambdalisue lambdalisue deleted the patch-LunarWatcher branch August 26, 2023 14:26
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.

None yet

3 participants