Add option to move editors with workbench.editor.moveToActiveGroupIfOpen

[jonathanjameswatson]

Adds the workbench.editor.revealIfOpenInActiveGroup configuration option, which simply changes workbench.editor.revealIfOpen to make any already-opened editor in a non-active editor group move to the active editor group on reveal. This makes opening an editor match the behavior of Emacs and Vim in a general and purely opt-in way.

Please let me know if there is anything I can do to help getting this PR merged.

Resolves #204942.

Demo:

2024-02-17.15-56-14.mp4

Edit: This configuration option is now called workbench.editor.moveToActiveGroupIfOpen

Demo by @aguynamedben:

https://app.screencast.com/e4b1UiiBoWHO0?tab=Details&conversation=X5VpzpDiGkj107qoKseGT4

[jonathanjameswatson]

@microsoft-github-policy-service agree

[aguynamedben]

Dude, this is amazing. I think this is going to help a lot of people coming from vim/neovim, Emacs, and other keyboard-centric editors.

The only other thing I can think of is to update Working without Tabs in the docs to something like:

---

(I think this fits best under the "Working without Tabs" section.)

Simulating Buffers

If you're coming from buffer-centric editors such as Emacs or vi, you can emulate that workflow with:

"workbench.editor.showTabs": none, // or 'single'
"workbench.editor.revealIfOpen": true,
"workbench.editor.revealIfOpenInActiveGroup": true,
"workbench.editor.closeEmptyGroups": false,

VS Code has a model where Editors live hierarchically within Editor Groups, and to split your screen you must use Editor Groups. Using revealIfOpen with revealIfOpenInActiveGroup causes Quick Open to move already-open Editors into the active Editor Group. This allows you to emulate buffer-centric environments—no tabs, split your screen, and you can view any Editor within any Editor Group without opening a file multiple times.

[jonathanjameswatson]

@aguynamedben

The only other thing I can think of is to update Working without Tabs in the docs to something like:

(I think this fits best under the "Working without Tabs" section.)

Simulating Buffers

If you're coming from buffer-centric editors such as Emacs or vi, you can emulate that workflow with:

"workbench.editor.showTabs": none, // or 'single' "workbench.editor.revealIfOpen": true, "workbench.editor.revealIfOpenInActiveGroup": true

VS Code has a model where Editors live hierarchically within Editor Groups, and to split your screen you must use Editor Groups. Using revealIfOpen with revealIfOpenInActiveGroup causes Quick Open to move already-open Editors into the active Editor Group. This allows you to emulate buffer-centric environments—no tabs, split your screen, and you can view any Editor within any Editor Group without opening a file multiple times.

I agree that we should add this in a PR in https://github.com/microsoft/vscode-docs if this PR gets accepted. I would also add that "workbench.editor.closeEmptyGroups": false could be good for an Emacs-like experience in the edge case of this feature moving an editor out of an editor group that only has one editor.

[aguynamedben]

@jonathanjameswatson Good call on closeEmptyGroups, I will prepare the docs PR so it's ready

[gjsjohnmurray]

I think moveToActiveGroupIfOpen would be a clearer name for the setting.

[jonathanjameswatson]

I think moveToActiveGroupIfOpen would be a clearer name for the setting.

OK, I will rename it quickly!

[jonathanjameswatson]

@gjsjohnmurray I have now renamed the configuration option and confirmed that it still works.

[bpasero]

I am not sure the editor group finder is the best place for this. It is not being used in all cases, only when the group is not specified explicitly.

We already have a location in our editor resolver where we look for an existing editor and move it conditionally:

vscode/src/vs/workbench/services/editor/browser/editorResolverService.ts

Line 505 in 9ccb0fa

const singleEditorPerResource = typeof selectedEditor.options?.singlePerResource === 'function' ? selectedEditor.options.singlePerResource() : selectedEditor.options?.singlePerResource;

This is for editors that can only be opened once for a resource, so I wonder if the option could apply here as well.

@lramos15 any objections?

[lramos15]

This is for editors that can only be opened once for a resource, so I wonder if the option could apply here as well.

This is sort of a special case that has to do with the inability to resolve the editor otherwise. I'm not sure the resolver should own user configurations about editor position since its main focus is just on editor types. Also then you wonder if other reveal related settings would move to the resolver? This is a small change so not a huge deal for now but I do worry if over time this will create code debt. Let me know your thoughts.

[bpasero]

I still think that this line is not 100% correct:

https://github.com/microsoft/vscode/pull/205442/files#diff-a9835b25a04cc268f3b704b319d5bfc15be4b0c85c2debe2708459b0726ea5feR161

We are in the process of trying to figure out which group to use and there we reference editorGroupService.activeGroup, but that is potentially not the group the user wants, for example when opening to the side. Thus I think this needs to be determined after the group has been figured out in the resolver or somewhere after the method was called.

[jonathanjameswatson]

It looks like you are right to say that this is not fully correct.

If the editor grid is:

Editor group 1: Editor A (active, visible), Editor B
Editor group 2: Editor C (visible)

Then opening editor B to the side will result in:

Editor group 1: Editor A (visible), Editor B
Editor group 2: Editor B (active, visible), Editor C

rather than:

Editor group 1: Editor A (visible)
Editor group 2: Editor B from editor group 1 (active, visible), Editor C

It definitely doesn't help that the line you mention only applies when preferredGroup is either undefined or ACTIVE_GROUP_TYPE, but I expect you are right that code outside of editorGroupFinder will still need to be changed even if this is fixed.

Something that I find interesting here is that if you have:

Editor group 1: Editor A (active, visible)
Editor group 2: Editor B, Editor C (visible)

and you open editor B to the side, you always seem to get:

Editor group 1: Editor A (active, visible)
Editor group 2: Editor B (visible), Editor C

even if workbench.editor.revealIfOpen is false. Is this the logic in the editor resolver that could be changed to work with moveToActiveGroupIfOpen?

[bethandtownes]

@jonathanjameswatson hi, thanks for this code change! I think this is going to make life a lot easier for vim/emacs users to onboard to vscode since will make vscode's editor concept to behave more like the buffer systems in those two editors.

@bpasero I saw this PR has been here for a while, wondering if there is anything we from open source community could do to help check this in?

[jonathanjameswatson]

@jonathanjameswatson hi, thanks for this code change! I think this is going to make life a lot easier for vim/emacs users to onboard to vscode since will make vscode's editor concept to behave more like the buffer systems in those two editors.

@bpasero I saw this PR has been here for a while, wondering if there is anything we from open source community could do to help check this in?

Hi, I do need to have another go at implementing this feature since the first attempt wasn't fully correct. If you would like to help, the first step would be to figure out a function that can be changed in a way that can handle all situations.

I think this could be done by putting some breakpoints in the editor resolver and experimenting with opening editors in different ways. For example, opening a definition to the side.

[jonathanjameswatson]

@bpasero I have now fixed the implementation of this feature by modifying the block of code that you linked to. I have checked that this feature works with opening to the side. It still works with editor group locking and having revealIfOpen enabled at the same time.

I made some changes to moveExistingEditorForResource which shouldn't affect its existing use case of handling singlePerResource editors. I have also documented that it should only take non-empty lists although perhaps we should also add an extra length check to the start. I also believe that the singlePerResource logic may currently ignore editor group locking. This behavior has been maintained in my PR but can be changed.

I check the configuration service in doResolveEditor now although it turns out there were already existing usages of the configuration service in editorResolverService. I could potentially change the function arguments here instead, although I think this would overall make the code style worse. Also, if you think it would be useful, I could add a change listener for moveToActiveGroupIfOpen so we don't call this.configurationService.getValue every time a file is opened?

[aguynamedben]

lgtm

[aguynamedben]

@bpasero Thank you for coaching through this change. Can you review? We need one more review for the check.

[aguynamedben]

@jonathanjameswatson Maybe we should change the title of the PR to "Add option workbench.editor.moveToActiveGroupIfOpen" and update the PR description with that option name too.

I'm testing this tonight and it works really nicely. It's working flawlessly over the past 60 minutes.

Demo: https://app.screencast.com/e4b1UiiBoWHO0?tab=Details&conversation=X5VpzpDiGkj107qoKseGT4

This is an important change for anybody coming from Emacs, Neovim, or other buffer-based editors. It's also important for users that don't wish to use the mouse and file tree to open files, as they can lean on Quick Open (Cmd-P) harder and it wont open 1-file-per-split. It also makes other options such as editor.showTabs": "single" more valuable, along with splitting. Given the small size of the change and the strong history of text editors having "recallable" buffers and keyboard navigation, I'd love to see this PR merged.

[aguynamedben]

The vscode-docs PR has been updated accordingly - microsoft/vscode-docs#7043

[bethandtownes]

@bpasero ^^ for review :) I think this feature is super useful for power users coming from Emacs/Vim community.

[maxgillett]

Really nice work @jonathanjameswatson :)

Any ETA on the second review? As others have stated, it would be nice to finally make the switch from Vim once this is merged.

[bethandtownes]

@bpasero ^^

[aguynamedben]

@bpasero This is a 48–line change generated by the community and discussed and submitted 10 months ago in February. @jonathanjameswatson and myself have done our best to test, respond to reviews/feedback, and provide documentation. Is there anything we can do to help get this option merged? I feel like we've done everything as correctly as possible, but it still hasn't been merged 10 months later.

[bpasero]

I appreciate the effort that went into this PR but in its current form, it does not seem finished for all the cases. I know I had initially suggested to add the logic close to moveExistingEditorForResource but now looking at it again, I realise that not all editors go through this method.

For example: take terminal editors. Opening them from quick pick will not go through the logic of moveExistingEditorForResource and as such, the new setting would not apply to terminals, which makes the experience inconsistent.

And I am still not 100% sure if the fact that the editorGroupFinder will return a different result vs. the result of opening the editor wouldn't introduce other bugs and inconsistencies.

Sorry that I do not have better news, but for this issue to move forward, it needs to be looked at more closely by someone from us and I think cannot be driven entirely by contributions.

[jonathanjameswatson]

I appreciate the effort that went into this PR but in its current form, it does not seem finished for all the cases. I know I had initially suggested to add the logic close to moveExistingEditorForResource but now looking at it again, I realise that not all editors go through this method.

For example: take terminal editors. Opening them from quick pick will not go through the logic of moveExistingEditorForResource and as such, the new setting would not apply to terminals, which makes the experience inconsistent.

And I am still not 100% sure if the fact that the editorGroupFinder will return a different result vs. the result of opening the editor wouldn't introduce other bugs and inconsistencies.

Sorry that I do not have better news, but for this issue to move forward, it needs to be looked at more closely by someone from us and I think cannot be driven entirely by contributions.

That's fair, I appreciate your concerns. I hope you can take the original issue #204942 into account when prioritising work next year, as I think it could really improve the developer experience for lots of users with different workflows. I'm always happy to help out in any way, should this work be picked up by Microsoft.