[plug-in] Implement inspect configuration method of Preferences API #4153
Conversation
akurinnoy
requested review from
benoitf,
evidolob,
akosyakov,
AndrienkoAleksandr and
mmorhun
akurinnoy
force-pushed
the
inspect-configuration
branch
from
64876f6 to
b8d8490
Compare
| @@ -30,6 +30,7 @@ import './preferences-monaco-contribution'; | |||
| export function bindPreferences(bind: interfaces.Bind, unbind: interfaces.Unbind): void { | |||
| unbind(PreferenceProvider); | |||
|
|
|||
| bindDefaultPreferenceProvider(bind); | |||
There was a problem hiding this comment.
why do we need an external function
seems like the two following are not using a function so maybe could just write there:
bind(PreferenceProvider).to(PreferenceSchemaProvider).inSingletonScope().whenTargetNamed(PreferenceScope.Default);
There was a problem hiding this comment.
I added this function to avoid further merge conflict with #3247.
| }, {}); | ||
| return result; | ||
| }, {}); | ||
| } catch (e) { } |
There was a problem hiding this comment.
silent catch ?
in which case does it fail ?
There was a problem hiding this comment.
It fails if data is undefined or null.
But, in this case
https://github.com/theia-ide/theia/blob/b8d8490740d50a97e32618d2d4d4711e49156ac2/packages/plugin-ext/src/plugin/preference-registry.ts#L213-L215
will fail as well. So, I'm going to refactor it.
|
@benoitf @akosyakov PR is updated |
| @@ -59,6 +60,10 @@ export function bindPreferenceSchemaProvider(bind: interfaces.Bind): void { | |||
| bindContributionProvider(bind, PreferenceContribution); | |||
| } | |||
|
|
|||
| export function bindDefaultPreferenceProvider(bind: interfaces.Bind): void { | |||
| bind(PreferenceProvider).to(PreferenceSchemaProvider).inSingletonScope().whenTargetNamed(PreferenceScope.Default); | |||
There was a problem hiding this comment.
There is already instance of PreferenceSchemaProvider provided via bindPreferenceSchemaProvider.
bindDefaultPreferenceProvider should not create a completely different instance for PreferenceProvider symbol.
You can just inject PreferenceSchemaProvider in plugin-contribution-handler:
@inject(PreferenceSchemaProvider)
private readonly preferenceSchemaProvider: PreferenceSchemaProvider;There was a problem hiding this comment.
fixed
BTW is it OK for you to use get PreferenceSchemaProvider instance like this:
https://github.com/theia-ide/theia/blob/f82f3adce449c46b6cacd5a5f61a31c319330c82/packages/core/src/browser/frontend-application-module.ts#L197-L199
| import cloneDeep = require('lodash.clonedeep'); | ||
|
|
||
| /* tslint:disable:no-any */ | ||
|
|
There was a problem hiding this comment.
There should be CQ for any copied code. BTW do you change it? I think we already have this code in Monaco, so maybe you delete it and just expose then you don't need CQ and we have less code to maintain.
There was a problem hiding this comment.
Unfortunately, that code from Monaco cannot be reused in plugin's part of API. In order to avoid opening a CQ, I got rid of copied code and implemented it on my own.
Signed-off-by: Oleksii Kurinnyi <[email protected]>
akurinnoy
force-pushed
the
inspect-configuration
branch
from
0cd49c0 to
f82f3ad
Compare
|
@akosyakov PR is updated, please review it again |
This PR adds the implementation of
inspectmethod without the support of multi-root workspaces.resolves #2289
Signed-off-by: Oleksii Kurinnyi [email protected]