Skip to content
This repository has been archived by the owner on Mar 22, 2024. It is now read-only.

Properly separate and define classic and extended editor #54

Merged
merged 9 commits into from
Oct 16, 2023

Conversation

kaisalmen
Copy link
Collaborator

@kaisalmen kaisalmen commented Oct 6, 2023

@montymxb when I reviewed the tutorial update I realized that the terminology around classic editor and vscode-api is not properly defined here. This PR should cleans this up; in the code and the documentation.

monaco-vscode-api is the foundation for everything here and therefore using something like vscodApi for one of the editor config/modes is misleading. This PR corrects this:

  • classic: Configure monaco-editor as you would when using it directly
  • Extended: Configure monaco-editor like a VSCode extension

In case of extended mode it is ensured textmate and theme services, plus the default themes are available when you use it.
userConfiguration is now only available in this mode as the regular editorOptions should be used in *classic mode.

I spent some time cleaning EditorAppBase no longer depending on any code from extending classes. This is now cleaner. Sorry this PR grew a lot bigger than initially intended, but it helps if code and description are in-line.

next release are available for testing:
https://www.npmjs.com/package/monaco-editor-wrapper/v/3.3.0-next.2
https://www.npmjs.com/package/@typefox/monaco-editor-react/v/2.3.0-next.2

This change will also make the Langium web template config easier as it removes the need for separate bundles, because everything required is configured on this level already. The latest commit there use the next version now.

The latest commit moves editorOptions, diffEditorOptions and overrideAutomaticLayout to EditorAppConfigBase. This allows to set editor parameters directly which may help overcome monaco-editor related configuration problems and wasn't present in extended mode before.

@kaisalmen kaisalmen requested a review from montymxb October 6, 2023 21:12
@kaisalmen kaisalmen force-pushed the vscode-themes-textmate branch from 955e0c8 to a662b88 Compare October 7, 2023 15:27
@kaisalmen kaisalmen changed the title Ensure vscode editor always comes with themes and textmate enabled. Properly separate and define classic and extended editor Oct 7, 2023
@kaisalmen kaisalmen force-pushed the vscode-themes-textmate branch from 8d8d07c to 49982eb Compare October 7, 2023 20:17
@kaisalmen kaisalmen force-pushed the vscode-themes-textmate branch from 791daab to 13c79ce Compare October 11, 2023 06:52
@kaisalmen kaisalmen force-pushed the vscode-themes-textmate branch from bb1a5e4 to ae25843 Compare October 13, 2023 09:10
Copy link
Member

@montymxb montymxb left a comment

Choose a reason for hiding this comment

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

Some comments & suggestions. I was not able to test this locally, yet, but if the built-in examples run as expected I would be up for approving & adding in. Since this is a pre-release that may be fine as is, allowing us to verify how this integrates in practice.

package.json Outdated Show resolved Hide resolved
packages/examples/CHANGELOG.md Outdated Show resolved Hide resolved
packages/examples/CHANGELOG.md Show resolved Hide resolved
packages/examples/src/wrapperAdvanced.ts Show resolved Hide resolved
packages/monaco-editor-react/CHANGELOG.md Show resolved Hide resolved
packages/monaco-editor-wrapper/CHANGELOG.md Show resolved Hide resolved
packages/monaco-editor-wrapper/README.md Show resolved Hide resolved
packages/monaco-editor-wrapper/src/editorAppBase.ts Outdated Show resolved Hide resolved
Copy link
Member

@montymxb montymxb left a comment

Choose a reason for hiding this comment

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

Approved. Changes look good, tested the new additions out locally as well and everything seems to be in order. Next step is trying out the preview release

packages/examples/wrapper_langium.html Outdated Show resolved Hide resolved
@kaisalmen kaisalmen merged commit f8edf51 into main Oct 16, 2023
2 checks passed
@kaisalmen kaisalmen deleted the vscode-themes-textmate branch October 16, 2023 20:01
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.

2 participants