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

Controls how lines should wrap of the editor #961

Merged
merged 3 commits into from
Feb 2, 2022

Conversation

oo6
Copy link
Contributor

@oo6 oo6 commented Jan 30, 2022

solve #848

Some variable names from vscode

image

@josevalim
Copy link
Contributor

Thank you @oo6! Before adding a global editor configuration, do you think this could be added directly to the editor instead? I think configuring it on a case-by-case basis would be more useful. :)

@oo6
Copy link
Contributor Author

oo6 commented Feb 2, 2022

Thanks for the review, I tried it.

Monaco editor can't add submenus in the context menu(monaco-editor#1947), so I'm only add Toggle word wrapping.

@josevalim @jonatanklosko what do you think? And should I remove the global config?

@josevalim
Copy link
Contributor

I like this a lot @oo6! Once you click, does it show in the select menu that it is “toggled”? If it doesn’t, maybe we should have “Enable word wrapping” and “Disable word wrapping” instead? Can we add keyboard shortcuts to it?

@oo6
Copy link
Contributor Author

oo6 commented Feb 2, 2022

Once you click, does it show in the select menu that it is “toggled”? If it doesn’t, maybe we should have “Enable word wrapping” and “Disable word wrapping” instead?

Use the latter, and display it according to the different conditions.

Can we add keyboard shortcuts to it?

Yes, which shortcuts do you recommend? I followed vscode always uses alt-z to Toggle word wrapping.

@josevalim
Copy link
Contributor

Yes, which shortcuts do you recommend? I followed vscode always uses alt-z to Toggle word wrapping.

Yes, we should use the same shortcut. Can we list it as part of the menu item too? :)

@josevalim
Copy link
Contributor

Yes, we should use the same shortcut. Can we list it as part of the menu item too? :)

Nevermind, I saw you already did it. :D Great job! Let's remove the global configuration and this is good to go!

@oo6
Copy link
Contributor Author

oo6 commented Feb 2, 2022

Let's remove the global configuration and this is good to go!

Done.

@jonatanklosko
Copy link
Member

Beautiful, just one comment and we can ship it!

@jonatanklosko
Copy link
Member

Thank you!!

@jonatanklosko jonatanklosko merged commit 6826939 into livebook-dev:main Feb 2, 2022
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.

3 participants