Bump to monaco-editor 0.20.0 #207
Conversation
CGNonofr
requested review from
akosyakov,
AlexTugarev,
asual,
azatsarynnyy,
BroKun,
gatesn,
johnwiseheart,
mofux,
rcjsuen and
RomanNikitenko
as code owners
|
Open in Gitpod - starts a development workspace for this pull request in code review mode and opens it in a browser IDE. |
CGNonofr
force-pushed
the
bump-monaco-20
branch
from
9a34499
to
9dc8806
Compare
CGNonofr
force-pushed
the
bump-monaco-20
branch
from
f2fb98a
to
16601c3
Compare
|
I'd like to see this merged in, is there something I can do to help? |
| @@ -105,6 +107,11 @@ export class MonacoLanguageClient extends BaseLanguageClient { | |||
| } | |||
| } | |||
|
|
|||
| public registerProposedFeatures() { | |||
| this.registerFeature(new CallHierarchyFeature(this)); | |||
There was a problem hiding this comment.
Should CallHierarchyFeature be registered? I don't think Monaco supports this feature, does it?
There was a problem hiding this comment.
Indeed, an additional component will be needed to make it work I guess.
But the SemanticTokensFeature also requires some tweaks to work so... and it seems pretty harmless to register an unsupported feature (especially this one).
What do you suggest? Having a method per proposed feature?
There was a problem hiding this comment.
@CGNonofr I was more asking to make sure I hadn't missed it while looking at the code.
I don't think it's the end of the world to register it. It just needs to be made clear that this is unsupported given that Monaco does not support it.
There was a problem hiding this comment.
Using a comment? or by renaming the method? or just by updating the documentation?
|
@CGNonofr Would it be all right if @RomanNikitenko pushes to this PR to address outstanding comments and merge it? Could you make sure that the PR is opened for collaborations with maintainers: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests |
|
@akosyakov No problem for me, it's already "opened for collaborations with maintainers"! Feel free to do whatever is needed to get it merged |
|
@CGNonofr @akosyakov
Is it correct from your point of view? |
|
@RomanNikitenko I think it is fine to catch up with LSP. It could be also necessary for the semantic highlighting support. One has to check which minimal version vscode-languageclient is required to support it. |
|
@akosyakov
for |
|
I think it should be fine. |
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
RomanNikitenko
force-pushed
the
bump-monaco-20
branch
from
73c2104
to
e33200f
Compare
OK, thanks, changed to 6.1.3 and 1.43.0 respectively. |
|
@RomanNikitenko You can merge it to produce |
|
@CGNonofr |
No description provided.