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

Support "when":"true" in 'commandPalette' menus #9188

Merged

Conversation

alvsan09
Copy link
Contributor

Adds a missing check for "true" on contributed "commandPalette" menus

Fixes: #9065

Signed-off-by: Alvaro Sanchez-Leon alvaro.sanchez-leon@ericsson.com

What it does

Fixes the issue described by #9065, where contributed 'commandPalette' menus including the "when" clause and set to "true"
don't get presented to the user.

How to test

In your test extension add the following commands and menus to your package.json

  1. Add some commands to a test plugin e.g.
		"commands": [
			{
				"command": "Alvs-extension.dosomething-true",
				"title": "Alvs do something - true",
				"category": "Alvs"
			},
			{
				"command": "Alvs-extension.dosomething-false",
				"title": "Alvs do something - false",
				"category": "Alvs"
			},
			{
				"command": "Alvs-extension.dosomething-undefined",
				"title": "Alvs do something  - undefined",
				"category": "Alvs"
			}
		]
  1. Add corresponding 'commandPalette' menus to the commands above
    e.g.
		  "menus": {
			"commandPalette": [ 
			  {
				"command": "Alvs-extension.dosomething-true",
				"when": "true"
			  },
			  {
				"command": "Alvs-extension.dosomething-false",
				"when": "false"
			  },
			  {
				"command": "Alvs-extension.dosomething-undefined"
			  }
			]
		  }
  1. Display the available commands via

Ctrl-shft-P and filter by 'Alvs'

  1. Before this fix you would only see one entry for 'undefined'
    After applying the fix you shall see two entries 'undefined' and 'true'

Review checklist

Reminder for reviewers

@alvsan09 alvsan09 force-pushed the CommandPaletteWhenTrue branch from cf67631 to 63fd5c8 Compare March 11, 2021 19:20
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed using the following plugin that the commands are properly enabled based on the when context:

Command When Result
Hello World (True) 'true' the command is correctly enabled
Hello World (False) 'false' the command is correctly disabled
Hello World (Undefined) undefined the command is correctly enabled
Hello World (Dynamic) editorHasSelection the command is correctly enabled when there is a selection in the editor

In the future I think we can likely re-use the code from vscode/contextkey.ts to handle more advanced use-cases and contexts. If might be low effort to re-use this functionality which is core to the enablement of commands, menus and views.

@vince-fugnitto vince-fugnitto added commands issues related to application commands vscode issues related to VSCode compatibility labels Mar 15, 2021
Adds a missing check for "true" on contributed "commandPalette" menus

Fixes: eclipse-theia#9065

Signed-off-by: Alvaro Sanchez-Leon <alvaro.sanchez-leon@ericsson.com>
@alvsan09 alvsan09 force-pushed the CommandPaletteWhenTrue branch from e6c1da2 to af78ad8 Compare March 18, 2021 20:03
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that the updated changes still work correctly with the plugin:

  • true
  • false
  • undefined
  • dynamic - toggleable through editorHasSelection

@vince-fugnitto vince-fugnitto merged commit 309fab4 into eclipse-theia:master Mar 22, 2021
@vince-fugnitto vince-fugnitto added this to the 1.12.0 milestone Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commands issues related to application commands vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

package.json when true not respected
2 participants