-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 'onTaskType' activation event #12431
Conversation
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.
The functionality looks quite good, I only have some minor comments 👍
I noticed some improvements that can be made in follow-up pull-requests:
- display a
+ Configure Task
entry when no tasks are configured (abovecontributed
items) - add
Show All Tasks...
entry at the bottom of the list - improve ordering of entries in a multiroot (sort by root then tasks)
- add
Go back ↩
functionality
1) To pass unresolved 'picks' promise to the monaco 'esm'. Resolving picks before calling monaco 'esm' results in a delay to show the quick pick widget (Giving no feedback to the user during the delay). 2) Update the signature of 'QuickInputService#pick' to handle 'QuickPickInput' picks as per latest monaco 'esm'. Signed-off-by: Alvaro Sanchez-Leon <[email protected]>
This brings the following benefits: 1) Activates specific plugins rather than all task provider plugins. This is achieved by presenting the 'provided' task types to the user and upon selection activate only the provider(s) for that specific selected task type. 2) The above avoids startup delays, and reduces the memory footprint on the extension host by only loading the task provider plugins which are selected. 3) Enables the use of plugins which support 'onTaskType' e.g. 'Gulp', 'Grunt', 'Jake'. NOTE: The above apply to extensions that have stopped using activation by the command 'workbench.action.tasks.runTask' in favor of 'onTaskType' activation. For backward compatibility the activation by the command mentioned above is still supported. Additionally, Selecting the option to configure tasks (e.g., Terminal/Configure tasks...) requires and triggers activation of all tasks providers. The solution introduces two pick levels for 'provided' tasks, one for the selection of the task type and the second one to resolve and show the corresponding tasks to the selected type which the user can then pick from. The following scenarios are impacted: 1) Terminal/Run Task (for a specific task type) a) Long resolution (e.g., many provided tasks) b) quick resolution (e.g., small number of provided tasks) 2) Terminal/Configure Tasks... 3) Command palette Quick Access (e.g. ?Task) Signed-off-by: Alvaro Sanchez-Leon <[email protected]>
The 'vscode.api' for creation of tasks has two constructors, one of them deprecated which does not require the 'scope' parameter. Some extensions (e.g., Jake) still use the deprecated api and therefore it is possible to receive 'Task's where 'scope' is undefined and causes an exception in 'Theia' which prevents the use of the related tasks. This change fixes it by using 'TaskScope.Workspace' as a default in such cases. Signed-off-by: Alvaro Sanchez-Leon <[email protected]>
e98920e
to
2d8c5fe
Compare
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.
LGTM 👍
I confirmed that:
- contributed tasks are now displayed
- contributed tasks work well, and show progress when loading (ex: gulp)
- plugins support
onTaskType
properly - the quick-access menu works for tasks
-
configure task...
works well, and displays the loaded tasks (ex: gulp)
What it does
Provides support for
onTaskType
activation eventsee #4199 (comment)
This brings the following benefits:
onTaskType
e.g. 'Gulp', 'Grunt', 'Jake'.NOTE: The above apply to extensions that have stopped using activation by the command
workbench.action.tasks.runTask
in favor ofonTaskType
activation. For backward compatibility the activationby the command mentioned above is still supported.
Additionally, Selecting the option to configure tasks (e.g., Terminal/Configure tasks...) requires and triggers activation
of all tasks providers.
The solution introduces two pick levels for 'provided' tasks,one for the selection of the task type and the second one to resolve and show the corresponding tasks to the selected type which the user can then pick from.
The following scenarios are impacted:
a) Long resolution (e.g., many provided tasks)
b) quick resolution (e.g., small number of provided tasks)
How to test
Prerequisites:
Activate
Auto Detect
preference forGulp
toon
From the:
Command palette -> type 'open user settings' -> click and then -> Navigate to (Extensions -> Gulp) and change 'Auto Detect' to 'on'
Activate
Auto Detect
preference forJake
toon
From the:
Command palette -> type 'open user settings' -> click and then -> Navigate to (Extensions -> Jake) and change 'Auto Detect' to 'on'
Use a clone of 'vscode' as the project to test
theia
with.vscode/settings.json
to delete the line:"gulp.autoDetect": "off"
.vscode/tasks.json
to remove the only "gulp" taskthis to prevent early activation of gulp.
Example test content:
Steps to test:
theia
and open the clonedvscode
repository mentioned above.DEBUG CONSOLE
using the filterload
onTaskType
activation has been loaded/activated i.e.gulp
,jake
,grunt
theia
instance run agulp
task i.e. Terminal / Run Task... => Selectgulp
=> wait for it to populate the plugin provided tasks on a second quick pick and select a task to run e.g. thevscode-api-test
DEBUG CONSOLE
observe thatgulp
was the only loaded / activated plugin.Jake
task.grunt
must have been activated with this action.Quick Access
i.e., From the command palette, type?
and then selecttask
gulp
,jake
(First selecting type, secondly selecting a provided task).Configure Task...
first and then again runningQuick Access
first.See test example video:
https://user-images.githubusercontent.com/76971376/232908518-580d56ce-fea2-4eac-b6ac-b018fbb33d20.mp4
Review checklist
Reminder for reviewers