feat: z fold bindings for lists (incl. files explorer) #1250

martin-braun · 2023-07-01T04:48:23Z

nvim-tree allows to copy files using c as well. Also, you can collapse all folders with W which is also a default on nvim-tree.

martin-braun · 2023-07-01T05:36:52Z

Unrelated pasting line after vi{ test for #116 blocks my contribution.

theol0403 · 2023-07-01T05:37:43Z

Don't worry about the tests, they are flaky.

martin-braun · 2023-07-01T05:38:28Z

@theol0403 How to re-trigger them?

theol0403 · 2023-07-01T05:39:18Z

I can merge on a failing test. Just ignore the results.

package.json

martin-braun · 2023-07-02T12:27:38Z

@justinmk I agree. Let's not waste more time here. Anybody is free to adjust maps, I just added zM for collapse, since this is reasonable.

martin-braun · 2023-07-02T14:58:50Z

@theol0403 I think it would be great to exclude "flaky" tests if they randomly fail so many times, what do you think? :)

theol0403 · 2023-07-02T22:14:53Z

I am trying to fix the tests but it is proving to be very frustrating - especially since some of the failures come from genuine timing bugs in the extension. For now, it is what it is. It's not the same flaky tests every time.

theol0403 · 2023-07-02T22:17:38Z

Just so we don't only have this one "z" binding that sticks out - is there any other z binding we could use that would make sense here? Even something like zz, zt for scrolling and any other fold+- bindings. I ask just because it feels awkward to introduce the z leader key to the possible tree bindings, but for there to only be one child.

theol0403 · 2023-07-02T22:18:26Z

Thanks for the PR btw! When I made the current bindings I picked something that seemed logical, but drawing inspiration from neotree or nvim-tree is in my opinion ideal. You are right however that we should not waste too much time, but complete and sane defaults will always provide a better experience.

martin-braun · 2023-07-03T03:44:21Z

@theol0403

but drawing inspiration from neotree or nvim-tree is in my opinion ideal.

So, you actually support my initial commit of mapping c and W? I'm torn apart, I don't mind. On one hand it feels good to apply defaults from popular tree plugins, on the other hand I have to give @justinmk props for mentioning that plugins come and go. What if we have the ultimate tree plugin tomorrow that uses c for a different behavior?

But ultimately, I don't mind really.

is there any other z binding we could use that would make sense here?

I agree, having only one z-binding is odd, but the result comes from the circumstances. Never used folding in Neovim, so lets elaborate together.

Vim is very feature rich when it comes to fold operations, VSCode doesn't offer enough workbench.file.action commands to substitute it properly. z<CR>/z+, z./zz, z- would be great to have, but I doubt I can scroll the file explorer window without changing its selection. There is list.focusPageDown and list.focusPageUp, but it will scroll down / up with cursor movement, which is not the same.

There are also no other folding operations for the files explorer. VSCode's fold actions live under editor. and apply for that only.

@justinmk

and c is the vim convention for "change", the complete opposite of yank.

I realize this is a loose point, because c has a lot of meanings in Vim, btw. :wincmd c will close a window and not change it, so a c mapping to copy files wouldn't be as controversial as it seems. Just a reminder.

All I wanted was to provide an alternative for copy and offer a collapse all (which is very useful to get clean on the files explorer) to play nice with nvim-tree compatibility.

Maybe we just drop it and let the user come up with their own, you decide.

theol0403 · 2023-07-03T04:02:43Z

I agree that is probably too minor of a topic to keep debating about.

So, you actually support my initial commit of mapping c and W?

I'm pretty indifferent, but I agree with @justinmk on those specific points. Both c and W seem pretty random. All I was trying to say was "it is good that someone is looking at the different setups and validating the current design". In the same vein, any existing bindings that don't make sense could be deleted.

I think something like c and W is something the user should add themselves 👍

Never used folding in Neovim, so lets elaborate together.

I think we could add za/zc/zo, using list.toggle/collapse/expand.

I think all we really need is for this pr to add za/zc/zo/zM, and potentially clean up any existing bindings that don't fit in with either vanilla nvim OR nvim-tree.

Thank you for your efforts on this btw, sorry for going back and forth so much on something trivial.

martin-braun · 2023-07-03T04:17:05Z

I think we could add za/zc/zo, using list.toggle/collapse/expand.

I will look into it later, kk.

martin-braun · 2023-07-04T05:25:31Z

@theol0403 zm vs zM, what is the difference? Also I can't replicate zO, zC, zA. Should they just not work or should they fallback to lowercase maps? Lastly, zR can't be mapped, because VSCode has no list.expandAll (btw).

theol0403 · 2023-07-04T06:10:36Z

@martin-braun I suppose bind zm and zM to the same thing. Same with zO/zC/zA. All lowercase fallbacks. And yeah, seems like zR is not possible.

justinmk

Should have a one-line note about Explorer mappings in the readme.md. It's not super obvious.

theol0403 · 2023-07-04T17:31:52Z

I can merge when ready.

…lorer-mappings

martin-braun · 2023-07-05T11:45:47Z

@theol0403 Please review once more. If it's cool: Let's go. :)

justinmk · 2023-07-05T12:00:34Z

README.md

+| <kbd>z</kbd> <kbd>o</kbd> / <kbd>z</kbd> <kbd>O</kbd> | `list.expand`                   |
+| <kbd>z</kbd> <kbd>c</kbd>                             | `list.collapse`                 |
+| <kbd>z</kbd> <kbd>C</kbd>                             | `list.collapseAllToFocus`       |
+| <kbd>z</kbd> <kbd>a</kbd> / <kbd>z</kbd> <kbd>A</kbd> | `list.toggleExpand`             |


To help with discovery, 1-2 line mention the readme is worth it. But the high-fidelity listing of every mapping is not worth it. Users should instead check vscode shortcuts UI to see exact mappings. The readme should just give a useful hint like:

In the vscode File Explorer some familiar vim commands are configured by default:

for navigation: j, k, gg, G

for expanding/collapsing directories: zo, zc, etc.

run the Preferences: Open Keyboard Shortcuts vscode command to see the complete list of keybindings.

This is a more maintainable pattern that we can follow in all sections of the readme, which will result in a less noisy, more maintainable, and still useful overview.

@justinmk Would you mind doing this? I don't know which place would be the best to insert these lines. I will revert my README.md changes appropriately later.

To clarify: I liked to mention each and every keybinding, because it was done for things like gg and even Enter, too. I'm not against summarizing the bindings, but it should go beyond this PR and should involve the removal of other bindings as well imho.

Ok, not a blocker

I do agree, but

until this is cleaned up, I'd rather include the list in the readme. I'll revert myself

unfortunately discovery in keybindings editor is difficult because there is no way to filter by extension

This reverts commit 09dd241.

This reverts commit 2e7747b.

martin-braun · 2023-07-05T13:20:15Z

Done!

This reverts commit 7d755fa.

This reverts commit 7ad2378.

theol0403 · 2023-07-05T16:30:10Z

Thanks for the PR!

martin-braun · 2023-07-05T18:32:09Z

Revert-Revert-Inception. You're welcome. :)

feat: collapse all and copy bindings for file explorer

178dc8f

Merge branch 'master' into file-explorer-mappings

ed5b015

justinmk reviewed Jul 1, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

justinmk reviewed Jul 1, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

fix: revert file explorer c map and adjust collapse map to zM

e9f9298

martin-braun closed this Jul 2, 2023

martin-braun reopened this Jul 2, 2023

justinmk approved these changes Jul 2, 2023

View reviewed changes

martin-braun changed the title ~~feat: collapse all and copy bindings for file explorer~~ feat: collapse all binding for file explorer Jul 2, 2023

feat: z fold bindings for general lists

a7782ea

martin-braun changed the title ~~feat: collapse all binding for file explorer~~ feat: z fold bindings for lists (incl. files explorer) Jul 4, 2023

Merge branch 'master' into file-explorer-mappings

5a703eb

justinmk approved these changes Jul 4, 2023

View reviewed changes

Martin Braun added 2 commits July 5, 2023 13:33

Merge commit '64ccf10e8df3cc04e674af876fbd71368994b556' into file-exp…

8d4bed7

…lorer-mappings

feat: alternative z fold bindings in lists

abbe9e7

docs: z fold list bindings

2e7747b

martin-braun requested a review from justinmk July 5, 2023 11:45

chore: removed unnecessary space in bindings doc

09dd241

justinmk reviewed Jul 5, 2023

View reviewed changes

Martin Braun added 2 commits July 5, 2023 14:07

Revert "chore: removed unnecessary space in bindings doc"

7ad2378

This reverts commit 09dd241.

Revert "docs: z fold list bindings"

7d755fa

This reverts commit 2e7747b.

justinmk approved these changes Jul 5, 2023

View reviewed changes

Martin Braun added 2 commits July 5, 2023 09:28

Revert "Revert "docs: z fold list bindings""

41bdf9c

This reverts commit 7d755fa.

Revert "Revert "chore: removed unnecessary space in bindings doc""

ecfb513

This reverts commit 7ad2378.

theol0403 merged commit a86bd36 into vscode-neovim:master Jul 5, 2023
5 checks passed

theol0403 mentioned this pull request Jul 5, 2023

chore(master): release 0.3.0 #1279

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: z fold bindings for lists (incl. files explorer) #1250

feat: z fold bindings for lists (incl. files explorer) #1250

martin-braun commented Jul 1, 2023

martin-braun commented Jul 1, 2023 •

edited

theol0403 commented Jul 1, 2023

martin-braun commented Jul 1, 2023

theol0403 commented Jul 1, 2023

martin-braun commented Jul 2, 2023

martin-braun commented Jul 2, 2023 •

edited

theol0403 commented Jul 2, 2023

theol0403 commented Jul 2, 2023

theol0403 commented Jul 2, 2023 •

edited

martin-braun commented Jul 3, 2023 •

edited

theol0403 commented Jul 3, 2023

martin-braun commented Jul 3, 2023

martin-braun commented Jul 4, 2023 •

edited

theol0403 commented Jul 4, 2023

justinmk left a comment

theol0403 commented Jul 4, 2023

martin-braun commented Jul 5, 2023 •

edited

justinmk Jul 5, 2023

martin-braun Jul 5, 2023 •

edited

justinmk Jul 5, 2023

theol0403 Jul 5, 2023

martin-braun commented Jul 5, 2023

theol0403 commented Jul 5, 2023

martin-braun commented Jul 5, 2023

feat: z fold bindings for lists (incl. files explorer) #1250

feat: z fold bindings for lists (incl. files explorer) #1250

Conversation

martin-braun commented Jul 1, 2023

martin-braun commented Jul 1, 2023 • edited

theol0403 commented Jul 1, 2023

martin-braun commented Jul 1, 2023

theol0403 commented Jul 1, 2023

martin-braun commented Jul 2, 2023

martin-braun commented Jul 2, 2023 • edited

theol0403 commented Jul 2, 2023

theol0403 commented Jul 2, 2023

theol0403 commented Jul 2, 2023 • edited

martin-braun commented Jul 3, 2023 • edited

theol0403 commented Jul 3, 2023

martin-braun commented Jul 3, 2023

martin-braun commented Jul 4, 2023 • edited

theol0403 commented Jul 4, 2023

justinmk left a comment

Choose a reason for hiding this comment

theol0403 commented Jul 4, 2023

martin-braun commented Jul 5, 2023 • edited

justinmk Jul 5, 2023

Choose a reason for hiding this comment

martin-braun Jul 5, 2023 • edited

Choose a reason for hiding this comment

justinmk Jul 5, 2023

Choose a reason for hiding this comment

theol0403 Jul 5, 2023

Choose a reason for hiding this comment

martin-braun commented Jul 5, 2023

theol0403 commented Jul 5, 2023

martin-braun commented Jul 5, 2023

martin-braun commented Jul 1, 2023 •

edited

martin-braun commented Jul 2, 2023 •

edited

theol0403 commented Jul 2, 2023 •

edited

martin-braun commented Jul 3, 2023 •

edited

martin-braun commented Jul 4, 2023 •

edited

martin-braun commented Jul 5, 2023 •

edited

martin-braun Jul 5, 2023 •

edited