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

customEditor toggle and save/preview keybinds #86505

Merged
merged 8 commits into from
Dec 10, 2019

Conversation

sverg1
Copy link
Contributor

@sverg1 sverg1 commented Dec 7, 2019

This PR aims to resolve #86504.

Tested on macOS High Sierra, Windows 10, and Ubuntu 18.04.3 LTS. Used README.md in test/ as a visual reference for keybinding and editor tests.

Toggle

By pressing CMD+E on Mac, or CTRL+E on Windows/Linux, the editor window opens a new tab in the same window that show either the default text editor or the custom Markdown preview editor, depending on the which editor was previously active editor. The toggle command does not get executed if in a context that does not have custom editors.

Save-to-Preview

By pressing CMD+Y on Mac, or CTRL+Y on Windows/Linux, the editor saves the changes—if any— of the text editor and switches to the custom editor. This command does not get executed if the context does not have custom editors. No changes are made if the custom editor is the currently active editor.

Caveats

  • I chose CMD+Y instead of CMD+S because I had a difficult time getting the command to execute with CMD+S. Experimenting with all the KeybindingWeight values in my custom save command changed nothing, so the next best thing was for me to use a different key altogether.
  • Using CTRL+E to toggle editors on Windows triggered to "Go to File..." command. Upon further inspection, CTRL+E is a secondary source-defined binding for that command but only for Windows (Mac and Linux had only the primary CTRL+P bound).

@msftclas
Copy link

msftclas commented Dec 7, 2019

CLA assistant check
All CLA requirements met.

@sverg1 sverg1 closed this Dec 7, 2019
@sverg1 sverg1 reopened this Dec 7, 2019
@mjbvz mjbvz added this to the December 2019 milestone Dec 7, 2019
Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

I think the expected behavior would be to have a single toggle command that does the following:

  • If a custom editor is focused, then replace it with the normal text editor
  • If a text editor is opened, then replace it with a custom editor (if one exists)

Let's get that working for unedited files first and then can figure out how to handle saves (because custom editors can also be edited)

@sverg1
Copy link
Contributor Author

sverg1 commented Dec 7, 2019

@mjbvz Would CMD+E still be the right way to go, given possible conflicting commands? If I don't have to worry about the secondary "Go to File" shortcut for Windows, that's fine, since the command runs otherwise.
I will scrap the save command and modify the toggle to replace the window as opposed to opening a new tab.

@octref octref requested a review from mjbvz December 9, 2019 16:09
Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Looking good. Let me know if you have any questions about my comments

src/vs/workbench/contrib/customEditor/browser/commands.ts Outdated Show resolved Hide resolved
src/vs/workbench/contrib/customEditor/browser/commands.ts Outdated Show resolved Hide resolved
src/vs/workbench/contrib/customEditor/browser/commands.ts Outdated Show resolved Hide resolved
src/vs/workbench/contrib/customEditor/browser/commands.ts Outdated Show resolved Hide resolved
@mjbvz mjbvz merged commit 7f05ebf into microsoft:master Dec 10, 2019
@mjbvz
Copy link
Collaborator

mjbvz commented Dec 10, 2019

Thanks! This will be in the first VS Code 1.42 insiders builds later this week

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toggle between default and custom editors
3 participants