Added key binding functionality for any task.

[cmrigney]

Closes #6550.

Update: The usage now is to define a command binding per @dbaeumer. Once a command binding is added to the task, it will be available for making it a keyboard shortcut.

Example task in tasks.json:

		{
			"taskName": "Do Something",
                        "suppressTaskName": true,
			"showOutput": "always",
			"commandBinding": {
				"id": "myproject.specialtask",
				"title": "Do something",
                                "category": "Optional category"
			}
		}

Then Do something will show up in the menu and to add a keyboard shortcut, do this in keybindings.json:

  {
    "key": "ctrl+r r",
    "command": "myproject.specialtask"
  }

[msftclas]

Hi @codyrigney92, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

[msftclas]

@codyrigney92, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

[dbaeumer]

@codyrigney92 thanks for the PR. I have a couple of suggestions:

• strings shown in the UI should never be concatenated using +. They always need to got through an nls.localize call. Think about a right to left language. Concatenating strings in the form you did doesn't work then. So code like nls.localize('RunTaskAction.label', "Run Task") + ' - ' + taskName must be written like this: nls.localize('RunTaskAction.label', "Run Task - {0}", taskName)
• Not all tasks in a tasks.json should be given a RunSpecificTaskAction. The reason being that for gulp and grunt we do auto task detection. If a gulp file has 20 tasks we will add 20 actions. IMO we should tag tasks in the tasks.json which should get its own action.

[cmrigney]

@dbaeumer Thanks I'll work through those suggestions. Can you elaborate what you mean by

IMO we should tag tasks in the tasks.json which should get its own action.

[dbaeumer]

@codyrigney92 I am thinking about something comparable to how commands are registered in the package.json of an extension. I think the stories should be aligned since it will allow us to even bind a task then to a menu item or an action presented in the editor. The Json should look something like this:

        {
            "taskName": "MyTask",
            "commandBinding": {
                "id": "the command id",
                "title": "the title",
                "category": "the category used in F1",
                "icon": "an optional icon"
            }
        }

Unfortunately we can't name the property command like in package.json since command is already taken for the OS command to run. You want to take a look at the IUserFriendlyCommand in menusExtensionPoint.ts.

If we have these command we can even register them with the MenuRegistry to bind tasks to UI pieces like context menus.

Let me know if this makes sense to you.

[cmrigney]

@dbaeumer I think this makes sense. So then would the key binding go next to commandBinding or would it go where it is now in the keybindings.json? Also, we don't have icons yet do we?

[dbaeumer]

@codyrigney92 the keybinding should go to the keybindings.json to ensure that we have one place where keybindings are defined. Icons could be resolved locally to the workspace if the path is relative.

[cmrigney]

@dbaeumer Ok. Going back to a previous comment, why wouldn't we want grunt tasks to be able to have key bindings? Your comment was this:

Not all tasks in a tasks.json should be given a RunSpecificTaskAction. The reason being that for gulp and grunt we do auto task detection.

[dbaeumer]

@codyrigney92 gulp and grunt tasks should be able to have key bindings. However if a gulp file has 20 tasks I don't want to see the 20 tasks in the Command Palette when pressing F1. So we shouldn't simply take all TaskDescription from the task service. Instead we should only generate commands for those that have the binding information.

May be some more background: for gulp and grunt we merge the tasks defined in tasks.json with the once defined in the corresponding file (gulpfile.js). We basically auto detect these tasks. So if the gulp file has a task 'xyz' it will be in the task service even if not listed in the tasks.json.

[cmrigney]

@dbaeumer Sorry I've been busy lately, I'll try to get to this soon.

[dbaeumer]

@cmrigney no problem. We wrap up September right now so this will have to wait for October

[visualperception]

Are we there yet?

[abarkine]

Is there any progress on this PR?

[cmrigney]

@20percent I'm going to attempt to finish this today.

[cmrigney]

I have this finished but I ran into some issues pushing it upstream. I'll have this updated tomorrow.

[cmrigney]

Ok I've pushed my new changes and updated my first comment on how to use it. @dbaeumer Can you double check me to make sure I didn't miss anything? I had trouble with the icon but I may have been misusing it.

Sorry this took so long and left some conflicts. Let me know if there's anything else I need to do with this pull. Thanks.

[dbaeumer]

@cmrigney I will see what I can do :-). Any change that you resolve the merge conflicts and the failing tests.

[cmrigney]

@dbaeumer I can resolve the conflicts but I'm not sure about the tests.

[cmrigney]

Whitespace is all cleaned up.

[cmrigney]

@dbaeumer Any idea why the AppVeyor test failed? The log is blank.

[dbaeumer]

Unfortunately I was not able to get this into November given the LSP work I had to do. For January we want to completely revisit the way tasks work. See the discussion here: #15179. I will make sure that this will make it into the new design.

[dbaeumer]

I implemented a new runner that executes task in the terminal. The benefit is that we can now execute more than one task in parallel. It is not on be default, but will be in the next release. The release notes for the next release will contain instructions how to play with this.

However this raises the questions that key binding will not be per task only. Consider the use case where a watch task compiler ts -> js and one task compiles less -> css and users want to bind one keybinding to both tasks and even might define the order in which they are executed (parallel, sequence, ...). Since it is not clear how this will be declared I didn't merge this into the Jan release. I envision currently something like a composite task to which a key binding can then be assigned. This wouldn't change the current design too mich. However not sure yet. If someone has ideas, please add them here.

[cmrigney]

@dbaeumer Is it possible to use this PR and modify it so that commandBinding optionally takes an array for multiple tasks?

[dbaeumer]

@cmrigney not sure I understand that correctly. The commandBinding property is currently local to a task. If it would take array, what would be the array type.

[cmrigney]

@dbaeumer Ah yes your right, my mistake. What about the command property of the keybindings? What if it took an array of strings?

 {
    "key": "ctrl+r r",
    "command": ["myproject.specialtask", "myproject.othertask"]
 }

Or maybe even allow each element in the array to be an object with other properties such as delay or whatever.

[dbaeumer]

Interesting idea. Currently I lean more towards something like this

{
"taskName": "SuperTask",
"tasks": ["taskOne", "taskTwo"]
}

So a task has either a command or a task array. Then the command binding would stay the same. But I am not sure if this is a nice solution.

[cmrigney]

That's a good idea. A task could execute other tasks. You would need to check for circular references though.

[dbaeumer]

Good point. The task execution engine I would say :-)

[dbaeumer]

@cmrigney started to work on this. A lot has change in the workbench which makes this now a lot easier. There are now command in the workbench first class and they can receive args from a keybinding. So this can even be done without any additional information in the tasks.json :-). See the cast attached. All I did are code changes and a keybinding definition. Hope this is what you expect:

[cmrigney]

@dbaeumer That's great! Is this in the current version or will it be in the next release?

[dbaeumer]

This will be in the next release and in the insider soon. Still on my private branch...

[dbaeumer]

I am closing this PR since it got implemented using a new / better way leveraging new API. It is available since 1.10.