Fix naming and availability in command palette of various commands #377
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| import { expect } from 'chai'; | ||
| import * as path from 'path'; | ||
| import * as fs from 'fs-extra'; | ||
|
|
||
| type CmdDecl = { | ||
| command: string; | ||
| when?: string; | ||
| title?: string; | ||
| } | ||
|
|
||
| describe('commands declared in package.json', function() { | ||
| const manifest = fs.readJsonSync(path.join(__dirname, '../../package.json')); | ||
| const commands = manifest.contributes.commands; | ||
| const menus = manifest.contributes.menus; | ||
|
|
||
| const disabledInPalette: Set<string> = new Set<string>(); | ||
|
|
||
| // These commands should appear in the command palette, and so | ||
| // should be prefixed with 'CodeQL: '. | ||
| const paletteCmds: Set<string> = new Set<string>(); | ||
|
|
||
| // These commands arising on context menus in non-CodeQL controlled | ||
| // panels, (e.g. file browser) and so should be prefixed with 'CodeQL: '. | ||
| const contribContextMenuCmds: Set<string> = new Set<string>(); | ||
|
|
||
| // These are commands used in CodeQL controlled panels, and so don't need any prefixing in their title. | ||
| const scopedCmds: Set<string> = new Set<string>(); | ||
| const commandTitles: { [cmd: string]: string } = {}; | ||
|
|
||
| commands.forEach((commandDecl: CmdDecl) => { | ||
| const { command, title } = commandDecl; | ||
| if (command.match(/^codeQL\./) | ||
| || command.match(/^codeQLQueryResults\./) | ||
| || command.match(/^codeQLTests\./)) { | ||
| paletteCmds.add(command); | ||
| expect(title).not.to.be.undefined; | ||
| commandTitles[command] = title!; | ||
| } | ||
| else if (command.match(/^codeQLDatabases\./) | ||
| || command.match(/^codeQLQueryHistory\./)) { | ||
| scopedCmds.add(command); | ||
| expect(title).not.to.be.undefined; | ||
| commandTitles[command] = title!; | ||
| } | ||
| else { | ||
| expect.fail(`Unexpected command name ${command}`); | ||
| } | ||
| }); | ||
|
|
||
| menus['explorer/context'].forEach((commandDecl: CmdDecl) => { | ||
| const { command } = commandDecl; | ||
| paletteCmds.delete(command); | ||
| contribContextMenuCmds.add(command); | ||
| }); | ||
|
|
||
| menus['editor/context'].forEach((commandDecl: CmdDecl) => { | ||
| const { command } = commandDecl; | ||
| paletteCmds.delete(command); | ||
| contribContextMenuCmds.add(command); | ||
| }); | ||
|
|
||
| menus.commandPalette.forEach((commandDecl: CmdDecl) => { | ||
| if (commandDecl.when === 'false') | ||
| disabledInPalette.add(commandDecl.command); | ||
| }); | ||
|
|
||
|
|
||
|
|
||
| it('should have commands appropriately prefixed', function() { | ||
| paletteCmds.forEach(command => { | ||
| expect(commandTitles[command], `command ${command} should be prefixed with 'CodeQL: ', since it is accessible from the command palette`).to.match(/^CodeQL: /); | ||
| }); | ||
|
|
||
| contribContextMenuCmds.forEach(command => { | ||
| expect(commandTitles[command], `command ${command} should be prefixed with 'CodeQL: ', since it is accessible from a context menu in a non-extension-controlled context`).to.match(/^CodeQL: /); | ||
| }); | ||
|
|
||
| scopedCmds.forEach(command => { | ||
| expect(commandTitles[command], `command ${command} should not be prefixed with 'CodeQL: ', since it is accessible from an extension-controlled context`).not.to.match(/^CodeQL: /); | ||
| }); | ||
| }); | ||
|
|
||
| it('should have the right commands accessible from the command palette', function() { | ||
| paletteCmds.forEach(command => { | ||
| expect(disabledInPalette.has(command), `command ${command} should be enabled in the command palette`).to.be.false; | ||
| }); | ||
|
|
||
| // Commands in contribContextMenuCmds may reasonbly be enabled or | ||
| // disabled in the command palette; for example, codeQL.runQuery | ||
| // is available there, since we heuristically figure out which | ||
| // query to run, but codeQL.setCurrentDatabase is not. | ||
|
|
||
| scopedCmds.forEach(command => { | ||
| expect(disabledInPalette.has(command), `command ${command} should be disabled in the command palette`).to.be.true; | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
| }); | ||
|
|
||
|
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had thought the difference between the
codeQLnamespace and thecodeQLDatabasesnamespace was that the latter accepted an existing database as an argument (and so was only applicable in the databases view). Whereas the former could be invoked in any context.I see you're disabling them as global commands, so maybe that's what we want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to be persuaded about choice of convention, but my intent so far about the
codeQL$FOO.$BARname structure (where$FOOis nonempty) was just about scoping to portions of the interface. Consider for examplecodeQLDatabases.sortByNamewhich does not take a database as an argument, and things likecodeQLQueryHistory.openQuerywhich are relevant to user events taking place in the query history view, but don't imply any particular constraints about the types of those handlers.