Skip to content

Conversation

@njkevlani
Copy link
Contributor

@njkevlani njkevlani commented Jul 3, 2018

Fixes #53372

What is done?

Added following Actions:

  • OpenRawDefaultKeybindingsAction
  • OpenRawDefaultKeybindingsAction

Added following Settings:

  • openDefaultKeybindings

What is remaining?

When keybindings are opened with keyindingsEditor, it shows User+Default keybindings regardless of what openDefaultKeybindings is set to(works for json file, though). I wanted to use already implemented fetch function, with @source:user, for this functionality. But I got some errors. Can you suggest how should I approach this @sandy081?

njkevlani added 2 commits July 3, 2018 21:59
Added workbench.settings.openDefaultKeybindings setting
@msftclas
Copy link

msftclas commented Jul 3, 2018

CLA assistant check
All CLA requirements met.

@sandy081 sandy081 self-requested a review July 4, 2018 08:41
@sandy081 sandy081 added this to the July 2018 milestone Jul 4, 2018
@sandy081 sandy081 added the config VS Code configuration, settings label Jul 4, 2018
@njkevlani
Copy link
Contributor Author

@sandy081 Done! Review please.

@njkevlani
Copy link
Contributor Author

Just putting a reminder here as there haven't been any activity for a while now. @sandy081

}

setInput(input: KeybindingsEditorInput, options: EditorOptions, token: CancellationToken): Thenable<void> {
this.searchWidget.setValue(input.defaultSearchValue);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change needed?

}
}

export class OpenRawUserKeybindingsAction extends Action {
Copy link
Member

Choose a reason for hiding this comment

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

We already have an action to open keybindings json file - OpenGlobalKeybindingsFileAction. So we do not need this action.

"textual" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true }
}
*/
const openDefaultKeybindings = !!this.configurationService.getValue('workbench.settings.openDefaultKeybindings');
Copy link
Member

Choose a reason for hiding this comment

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

Move this line to inside if(textual) block

}
return this.editorService.openEditor(this.instantiationService.createInstance(KeybindingsEditorInput), { pinned: true }).then(() => null);

const keybindingsEditorInput = this.instantiationService.createInstance(KeybindingsEditorInput);
Copy link
Member

Choose a reason for hiding this comment

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

I do not think openDefaultKeybindings setting should interfere in Keybindings editor. This setting should decide to open or not the default keybindings file when you open keybindings as a file instead of editor.

registry.registerWorkbenchAction(new SyncActionDescriptor(OpenGlobalSettingsAction, OpenGlobalSettingsAction.ID, OpenGlobalSettingsAction.LABEL), 'Preferences: Open User Settings', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(OpenGlobalKeybindingsAction, OpenGlobalKeybindingsAction.ID, OpenGlobalKeybindingsAction.LABEL, { primary: KeyChord(KeyMod.CtrlCmd | KeyCode.KEY_K, KeyMod.CtrlCmd | KeyCode.KEY_S) }), 'Preferences: Open Keyboard Shortcuts', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(OpenRawDefaultKeybindingsAction, OpenRawDefaultKeybindingsAction.ID, OpenRawDefaultKeybindingsAction.LABEL), 'Preferences: Open Raw Default Settings', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(OpenRawUserKeybindingsAction, OpenRawUserKeybindingsAction.ID, OpenRawUserKeybindingsAction.LABEL), 'Preferences: Open Raw Default Settings', category);
Copy link
Member

Choose a reason for hiding this comment

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

Same as previous comment, This is not needed.

return this.getEditableSettingsURI(ConfigurationTarget.USER);
}

get userKeybindingsResource(): URI {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this?

return this.editorService.openEditor({ resource: this.defaultKeybindingsResource });
}

openRawUserKeybindings(): TPromise<IEditor> {
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed

switchSettings(target: ConfigurationTarget, resource: URI): TPromise<void>;
openGlobalKeybindingSettings(textual: boolean): TPromise<void>;
openRawDefaultKeybindings(): TPromise<IEditor>;
openRawUserKeybindings(): TPromise<IEditor>;
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed

return otherInput instanceof KeybindingsEditorInput;
}

setDefaultSearchValue(defaultSearchValue = ''): void {
Copy link
Member

Choose a reason for hiding this comment

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

Not needed

Reverted changes regareding kebindingsEditor and removed some actions.
@njkevlani
Copy link
Contributor Author

I have pushed requested changes. Review please, @sandy081

registry.registerWorkbenchAction(new SyncActionDescriptor(OpenSettings2Action, OpenSettings2Action.ID, OpenSettings2Action.LABEL), 'Preferences: Open Settings (Preview)', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(OpenGlobalSettingsAction, OpenGlobalSettingsAction.ID, OpenGlobalSettingsAction.LABEL), 'Preferences: Open User Settings', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(OpenGlobalKeybindingsAction, OpenGlobalKeybindingsAction.ID, OpenGlobalKeybindingsAction.LABEL, { primary: KeyChord(KeyMod.CtrlCmd | KeyCode.KEY_K, KeyMod.CtrlCmd | KeyCode.KEY_S) }), 'Preferences: Open Keyboard Shortcuts', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(OpenRawDefaultKeybindingsAction, OpenRawDefaultKeybindingsAction.ID, OpenRawDefaultKeybindingsAction.LABEL), 'Preferences: Open Raw Default Settings', category);
Copy link
Member

Choose a reason for hiding this comment

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

Label should be Preferences: Open Default Keyboard Shortcuts File

@sandy081
Copy link
Member

@njkevlani LGTM. Thanks for the PR

@sandy081 sandy081 merged commit d4c2ccd into microsoft:master Jul 24, 2018
@njkevlani
Copy link
Contributor Author

Thanks for accepting!! @sandy081

@njkevlani njkevlani deleted the feature-#53372 branch July 24, 2018 15:08
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

config VS Code configuration, settings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workbench.settings.openDefaultKeybindings && workbench.action.openRawDefaultKeybindings

4 participants