Adopt latest list navigation support #3432

joaomoreno · 2019-01-31T14:38:31Z

What this PR does / why we need it:

This PR introduces a new keybinding when in the list context (/) which invokes the list.toggleKeyboardNavigation in order to use the new upcoming keyboard navigation feature in VS Code 1.31.

The introduction is backwards compatible since the keybinding is only enabled if listSupportsKeyboardNavigation is true which is the case only from 1.31 onwards.

Special notes for your reviewer:

Related VS Code issue: microsoft/vscode#66995

This should be merged before VS Code 1.31 releases.

jpoon · 2019-01-31T22:34:11Z

extension.ts

@@ -360,6 +360,8 @@ export async function activate(context: vscode.ExtensionContext) {
  }

  await Promise.all([
+    // This is in order to disable automatic keyboard navigation in lists
+    vscode.commands.executeCommand('setContext', 'listAutomaticKeyboardNavigation', false),


We ended up wrapping this as we've found that setContext sometimes takes awhile to complete. It doesn't make much difference in this context, but to follow with the pattern, can you call:

await VsCodeContext.Set('listAutomaticKeyboardNavigation', false);

jpoon · 2019-01-31T22:35:07Z

package.json

@@ -718,5 +723,10 @@
    "tslint": "5.12.1",
    "typescript": "3.2.4",
    "vscode": "1.1.28"
+  },
+  "__metadata": {


Ohh, secret fields -- what is this?

D'oh! Can't believe I let this slip. 🤦‍♂️ Just some gallery metadata.

jpoon

Yikes, you'll likely have conflicts -- I just moved some stuff around earlier today.

joaomoreno · 2019-02-01T06:25:01Z

No worries, I'll push new commits and resolve asap.

…tion

joaomoreno · 2019-02-01T08:36:51Z

@jpoon Alright, merged and cleaned up! 👍

TravisBuddy · 2019-02-01T08:37:36Z

Travis tests have failed

Hey @joaomoreno,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

Node.js: 8

View build log

if [[ $(git diff-index HEAD -- *.js *.ts *.md) ]]; then git diff; echo "Prettier Failed. Run `gulp forceprettier` and commit changes to resolve."; exit 1; fi

diff --git a/extension.ts b/extension.ts
index f237e13..ef7857b 100644
--- a/extension.ts
+++ b/extension.ts
@@ -116,14 +116,14 @@ export async function activate(context: vscode.ExtensionContext) {
       changeEvent.contentChanges.length === 1 &&
       changeEvent.contentChanges[0].text === '' &&
       changeEvent.contentChanges[0].range.start.line !==
-      changeEvent.contentChanges[0].range.end.line;
+        changeEvent.contentChanges[0].range.end.line;
 
     const textWasAdded = changeEvent =>
       changeEvent.contentChanges.length === 1 &&
       (changeEvent.contentChanges[0].text === '\n' ||
         changeEvent.contentChanges[0].text === '\r\n') &&
       changeEvent.contentChanges[0].range.start.line ===
-      changeEvent.contentChanges[0].range.end.line;
+        changeEvent.contentChanges[0].range.end.line;
 
     if (textWasDeleted(event)) {
       globalState.jumpTracker.handleTextDeleted(event.document, event.contentChanges[0].range);
diff --git a/test/configuration/configuration.test.ts b/test/configuration/configuration.test.ts
index 0bdf8e3..f91670c 100644
--- a/test/configuration/configuration.test.ts
+++ b/test/configuration/configuration.test.ts
@@ -78,7 +78,7 @@ suite('Configuration', () => {
   });
 
   test('neovim disabled on missing path', async () => {
-    assert.equal(false, srcConfiguration.configuration.enableNeovim)
+    assert.equal(false, srcConfiguration.configuration.enableNeovim);
   });
 
   newTest({
Prettier Failed. Run [08:36:50] Using gulpfile ~/build/VSCodeVim/Vim/gulpfile.js
[08:36:50] Starting 'forceprettier'...
[08:37:02] Finished 'forceprettier' after 13 s and commit changes to resolve.

TravisBuddy Request Identifier: 8cb8ac10-25fc-11e9-a660-67a7bedcbb29

joaomoreno · 2019-02-01T08:40:47Z

Ahah I even created a commit which left those bad whitespaces in simply because I didn't want more changes in the PR. But sure, bot, I can leave them in!

jpoon · 2019-02-01T14:37:02Z

test/configuration/configuration.test.ts

@@ -78,7 +78,7 @@ suite('Configuration', () => {
  });

  test('neovim disabled on missing path', async () => {
-    assert.equal(false, srcConfiguration.configuration.enableNeovim)
+    assert.equal(false, srcConfiguration.configuration.enableNeovim);


Yikes, thanks for fixing this.

joaomoreno · 2019-02-01T15:03:32Z

Thanks @jpoon! 🙏

bcpugh · 2019-02-06T21:30:15Z

thanks guys! slightly off-topic:
any advice on setting listSupportsKeyboardNavigation = false without implementing VSCodeContext.set() inside of an extension (i.e. a settings.json entry?)

joaomoreno · 2019-02-07T06:34:36Z

@bcpugh Do you have single-letter keybindings for lists regardless of using VIM?

FelikZ · 2019-02-07T10:58:09Z

So when I am focused on (for example) file explorer, vim mode enabled and hit / then it should enter a search mode?

I did not get it how it works.

Because I have updated VSCode and vim plugin and whenever I hit / I got the old behaviour (which is good).

joaomoreno · 2019-02-07T11:30:59Z

What was the old behaviour?

FelikZ · 2019-02-07T11:47:49Z

@joaomoreno
Old:

Focus on editor, hit / - toggles vim search
Focus on file explorer, hit / - nothing expected to happen
New:
Focus on editor, same behaviour
Focus on file explorer, hit / - filter/highlight / char (new feature)

So my question is, what this PR actually did, since in file editor it is the same behaviour as before and in file explorer (other lists) vim is not active there (was it?).

joaomoreno · 2019-02-07T12:14:14Z

VIM sets hjkl as keybindings to navigate in file explorer.

FelikZ · 2019-02-07T12:29:15Z

Thank you, it is clear now! Awesome job!

bcpugh · 2019-02-07T12:44:25Z

@joaomoreno

We lost functionality for key binding of “a” to create new file when the file tree is focused (and other similar bindings). Only some of our team uses VIM, but I wanted to ask here since the latest VScode release notes pointed us here. Can we somehow revert back to the old behavior without installing an extension like VIM?

joaomoreno · 2019-02-07T12:57:40Z

@bcpugh My recommendation would be to simply switch to a keybinding with modifier key, eg Ctrl N.

asilvadesigns · 2019-02-07T15:28:52Z

I'm very excited about this feature, awesome work, but it does not work for me.
With VIM enabled, and workbench.list.keyboardNavigation = "simple", focusing on the tree and typing "/" does not enter highlight or filter mode. What am I missing, I'm not sure to enable this?

nomad-software · 2019-02-07T18:51:51Z

How can I disable this feature added by this pull request?

macksta · 2019-02-11T04:01:02Z

This is an epic feature!! Good work!!

Molunerfinn · 2019-02-14T02:39:53Z

@asilvadesigns Hi~ I have found how to fix your problem!
You just need to set the
"workbench.list.keyboardNavigation": "filter" or
"workbench.list.keyboardNavigation": "highlight"
Since list.toggleKeyboardNavigation is to toggle navigation mode to filter or highlight. While you just set to simple, so the navigation mode is just simple after toggling(whether it is on or off).

controversial · 2019-02-28T15:16:31Z

Is there a way to disable the tree keyboard navigation without installing a plugin?

nomad-software · 2019-02-28T15:44:33Z

This is a really annoying change, can we please have a way of disabling this?

thomazmoura · 2019-03-02T23:11:33Z

Is there a way to disable the tree keyboard navigation without installing a plugin?

This is a really annoying change, can we please have a way of disabling this?

There is no need to install any plugin to disable this feature. If you just want to turn off the tree widget completely, use this setting on VS Code:

"workbench.list.keyboardNavigation": "simple"

joaomoreno · 2019-03-04T08:21:17Z

Is there a way to disable the tree keyboard navigation without installing a plugin?

This is a really annoying change, can we please have a way of disabling this?

I don't understand guys. This PR exists in order not to make keyboard filtering annoying for VIM users, who use hjkl keys to navigate in lists/trees.

What is annoying about this?

nomad-software · 2019-03-04T13:47:53Z

@joaomoreno because you're are forcing your way of working on other people. This has nothing to do with Vim, this is an editor change that alters the tree view and not an optional one at that so we all have to have it now! When I install a Vim plugin for any given editor, I expect it to give me Vim keybindings for the editor pane, not to start changing things outside of that unless it's strictly opt-in.

I wish the maintainer of this repo would limit pull requests to implement actual features of Vim and stop straying into altering the actual editor itself.

joaomoreno · 2019-03-04T13:55:32Z

@nomad-software Can you explain me in detail what currently doesn't work for you?

thomazmoura · 2019-03-08T16:23:22Z

@joaomoreno because you're are forcing your way of working on other people. This has nothing to do with Vim, this is an editor change that alters the tree view and not an optional one at that so we all have to have it now! When I install a Vim plugin for any given editor, I expect it to give me Vim keybindings for the editor pane, not to start changing things outside of that unless it's strictly opt-in.

I wish the maintainer of this repo would limit pull requests to implement actual features of Vim and stop straying into altering the actual editor itself.

@nomad-software I believe there is misunderstanding going on here about what this PR is about. It fixes the feature the VS Code team added to do VS Code and that was conflicting with VIM users. It doesn't add any feature to VS Code itself - the feature that adds the tree widget came directly from the VS Code team on their repo and it's not directly influenced by this repo or related to this PR.

And although I agree it might have been a better choice to leave it as opt-in (or at least easier to turn off after the upgrade) it's still opt-out (as most VS Code features are). Just check my former answer:

There is no need to install any plugin to disable this feature. If you just want to turn off the tree widget completely, use this setting on VS Code:
"workbench.list.keyboardNavigation": "simple"

So just to try to make things clearer: if you don't use or want to use VIM this PR does not affect you any way and there's no need or point in installing this extension. And if you want to disable the feature added to VS Code on 1.31 just use the code above.

adopt list navigation support

574440b

This was referenced Jan 31, 2019

Keyboard actions in file explorer not working and is stuck in fancy tree filter mode microsoft/vscode#66995

Closed

no longer can assign single letter keyboard shortcuts microsoft/vscode#67550

Closed

jpoon requested changes Jan 31, 2019

View reviewed changes

joaomoreno added 3 commits February 1, 2019 09:29

remove gallery metadata

beb8b06

Merge remote-tracking branch 'upstream/master' into adopt-list-naviga…

f99f489

…tion

whitespace

4185afc

forceprettier

6c6c705

jpoon reviewed Feb 1, 2019

View reviewed changes

jpoon approved these changes Feb 1, 2019

View reviewed changes

jpoon merged commit 22b826c into VSCodeVim:master Feb 1, 2019

joaomoreno deleted the adopt-list-navigation branch February 1, 2019 15:03

asilvadesigns mentioned this pull request Feb 7, 2019

Tree navigation does not work #3466

Open

joaomoreno mentioned this pull request Feb 7, 2019

Tree filtering breaks atom-keybindings microsoft/vscode#68083

Closed

joaomoreno mentioned this pull request Feb 7, 2019

Add compatibility to VSCode v1.31.0 microsoft/vscode-atom-keybindings#57

Merged

davewasmer mentioned this pull request Feb 7, 2019

List keyboard navigation broken in VS Code 1.31.0 #3474

Open

bpasero mentioned this pull request Feb 9, 2019

Workbench › List: Keyboard Navigation settings polish microsoft/vscode#66900

Closed

joaomoreno mentioned this pull request Feb 26, 2019

Type to filter doesn't work in tree when using Vim extension microsoft/vscode#69402

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adopt latest list navigation support #3432

Adopt latest list navigation support #3432

joaomoreno commented Jan 31, 2019

jpoon Jan 31, 2019

jpoon Jan 31, 2019

joaomoreno Feb 1, 2019

jpoon left a comment

joaomoreno commented Feb 1, 2019 •

edited

Loading

joaomoreno commented Feb 1, 2019

TravisBuddy commented Feb 1, 2019

joaomoreno commented Feb 1, 2019

jpoon Feb 1, 2019

joaomoreno commented Feb 1, 2019 •

edited

Loading

bcpugh commented Feb 6, 2019

joaomoreno commented Feb 7, 2019

FelikZ commented Feb 7, 2019

joaomoreno commented Feb 7, 2019

FelikZ commented Feb 7, 2019

joaomoreno commented Feb 7, 2019

FelikZ commented Feb 7, 2019

bcpugh commented Feb 7, 2019

joaomoreno commented Feb 7, 2019

asilvadesigns commented Feb 7, 2019

nomad-software commented Feb 7, 2019

macksta commented Feb 11, 2019

Molunerfinn commented Feb 14, 2019

controversial commented Feb 28, 2019

nomad-software commented Feb 28, 2019

thomazmoura commented Mar 2, 2019

joaomoreno commented Mar 4, 2019

nomad-software commented Mar 4, 2019

joaomoreno commented Mar 4, 2019 •

edited

Loading

thomazmoura commented Mar 8, 2019

Adopt latest list navigation support #3432

Adopt latest list navigation support #3432

Conversation

joaomoreno commented Jan 31, 2019

jpoon Jan 31, 2019

Choose a reason for hiding this comment

jpoon Jan 31, 2019

Choose a reason for hiding this comment

joaomoreno Feb 1, 2019

Choose a reason for hiding this comment

jpoon left a comment

Choose a reason for hiding this comment

joaomoreno commented Feb 1, 2019 • edited Loading

joaomoreno commented Feb 1, 2019

TravisBuddy commented Feb 1, 2019

Travis tests have failed

Node.js: 8

TravisBuddy Request Identifier: 8cb8ac10-25fc-11e9-a660-67a7bedcbb29

joaomoreno commented Feb 1, 2019

jpoon Feb 1, 2019

Choose a reason for hiding this comment

joaomoreno commented Feb 1, 2019 • edited Loading

bcpugh commented Feb 6, 2019

joaomoreno commented Feb 7, 2019

FelikZ commented Feb 7, 2019

joaomoreno commented Feb 7, 2019

FelikZ commented Feb 7, 2019

joaomoreno commented Feb 7, 2019

FelikZ commented Feb 7, 2019

bcpugh commented Feb 7, 2019

joaomoreno commented Feb 7, 2019

asilvadesigns commented Feb 7, 2019

nomad-software commented Feb 7, 2019

macksta commented Feb 11, 2019

Molunerfinn commented Feb 14, 2019

controversial commented Feb 28, 2019

nomad-software commented Feb 28, 2019

thomazmoura commented Mar 2, 2019

joaomoreno commented Mar 4, 2019

nomad-software commented Mar 4, 2019

joaomoreno commented Mar 4, 2019 • edited Loading

thomazmoura commented Mar 8, 2019

joaomoreno commented Feb 1, 2019 •

edited

Loading

joaomoreno commented Feb 1, 2019 •

edited

Loading

joaomoreno commented Mar 4, 2019 •

edited

Loading