Refactor to hide VS Code interfaces behind core services #9727
Conversation
| @@ -14,6 +14,8 @@ | |||
| * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 | |||
| ********************************************************************************/ | |||
|
|
|||
| const { QuickPickItem } = require('@theia/core/lib/browser'); | |||
There was a problem hiding this comment.
@tsmaeder I believe it should be inside the describe('file-search'), line 26.
| protected readonly quickAccessRegistry: QuickAccessRegistry; | ||
|
|
||
| constructor(@inject(KeybindingRegistry) protected readonly keybindingRegistry: KeybindingRegistry) { | ||
| } |
There was a problem hiding this comment.
do we really need an empty constructor?
can we use the property injection here instead?
vince-fugnitto
added
monaco
| super(); | ||
| this.initContainer(); | ||
| this.initController(); | ||
| this.monacoService = new MonacoQuickInputImplementation(this.contextKeyService); |
There was a problem hiding this comment.
Shouldn't we use inversify to inject the instance instead?
There was a problem hiding this comment.
The idea was that this is an implementation detail of the MonacoQuickInputService, but since we're using the same class over in MonacoEditorProvider, that is not true anyway.
The VS Code code can handle multiple instances no problem, but we don't need more than one instance, so using the one from the container seems the right thing.
tsmaeder
force-pushed
the
8969_refactor_quickinput
branch
from
9a1f71e to
dfbc685
Compare
|
Addressed review comments and rebased. Now (desperately ;-)) seeking for approval. 🥺 |
There was a problem hiding this comment.
@tsmaeder do you mind rebasing the pull-request, I'd be happy to perform another review so we can benefit from the cleanup following the upgrade 👍
Signed-off-by: Thomas Mäder <tmader@redhat.com>
Signed-off-by: Thomas Mäder <tmader@redhat.com>
Signed-off-by: Thomas Mäder <tmader@redhat.com>
Signed-off-by: Thomas Mäder <tmader@redhat.com>
Signed-off-by: Thomas Mäder <tmader@redhat.com>
Signed-off-by: Thomas Mäder <tmader@redhat.com>
tsmaeder
force-pushed
the
8969_refactor_quickinput
branch
from
595fa37 to
70393dc
Compare
| hideItem(label: string): void { } | ||
| showItem(label: string): void { } | ||
|
|
||
| readonly prefix = 'view '; |
There was a problem hiding this comment.
Leftover? We have QuickViewService.PREFIX.
Signed-off-by: Thomas Mäder <tmader@redhat.com>
…ia#9727) Refactor to hide VS Code interfaces behind core services Signed-off-by: Thomas Mäder <tmader@redhat.com>
Signed-off-by: Thomas Mäder tmader@redhat.com
What it does
This PR refactors the
QuickInputandQuickAccessservices introduced in #9154 in order to completely hide the types imported from VS Code.In particular, this allows to write the all the related contributions in terms of a single implementation of quick access and quick input. Also, the quick access registry implementation allows to use dependency injection in the regular Theia way.
This refactoring was deferred from the original PR in order to facilitate merging it earlier.
How to test
We should do the same testing as for #9154
Review checklist
Reminder for reviewers