Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the CommandQuickAccess provider #176682

Merged
merged 3 commits into from Mar 9, 2023
Merged

Use the CommandQuickAccess provider #176682

merged 3 commits into from Mar 9, 2023

Conversation

lramos15
Copy link
Member

@lramos15 lramos15 commented Mar 9, 2023

Appears the MenuRegistry is missing editor commands for the command palette so commands like Format Document are not available.

Maybe we should think about switching to the CommandRegistry at some point, but for now the command palette has nice names for common commads which makes finding what you want for development easy.

@lramos15 lramos15 enabled auto-merge (squash) March 9, 2023 19:41
@lramos15 lramos15 self-assigned this Mar 9, 2023
@VSCodeTriageBot VSCodeTriageBot added this to the March 2023 milestone Mar 9, 2023
Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

Moving from platform to workbench means this is no longer available in Monaco standalone.

@lramos15
Copy link
Member Author

lramos15 commented Mar 9, 2023

Moving from platform to workbench means this is no longer available in Monaco standalone.

This is a command I introduced to help me with some debugging related efforts. It was never intended to work in standalone monaco. Additionally, this command has not shipped in any stable version and is prefixed with _ which means internal so breaking changes are allowed.

TylerLeonhardt
TylerLeonhardt previously approved these changes Mar 9, 2023
@TylerLeonhardt
Copy link
Member

FYI @bpasero would love your thoughts

@lramos15 lramos15 merged commit c9c6c49 into main Mar 9, 2023
6 checks passed
@lramos15 lramos15 deleted the lramos15/nuclear-dinosaur branch March 9, 2023 22:22
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

I commented inline.

@@ -127,6 +124,13 @@ export class CommandsQuickAccessProvider extends AbstractEditorCommandsQuickAcce
}));
}

public getCommandInfo(): Array<ICommandQuickPick> {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public getCommandInfo(): Array<ICommandQuickPick> {
getCommandInfo(): Array<ICommandQuickPick> {

@@ -227,4 +231,15 @@ export class ClearCommandHistoryAction extends Action2 {
}
}

//#region --- Register a command to get all actions from the command palette
CommandsRegistry.registerCommand('_getAllCommands', async function (accessor: ServicesAccessor) {
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? It is somewhat ugly that we instantiate a UI component here only to get to the commands. Maybe the functionality for getting the commands should better be extracted to its own function or class?

lramos15 added a commit that referenced this pull request Mar 15, 2023
lramos15 added a commit that referenced this pull request Mar 15, 2023
* Revert "Use the CommandQuickAccess provider (#176682)"

This reverts commit c9c6c49.

* Move _getAllCommands to be when conditionless
@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 2023
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.

None yet

4 participants