access tasks configs as preferences #6268

elaihau · 2019-09-26T10:52:35Z

In current Theia, the task extension is responsible for reading,
writing, and watching tasks.json files. With this change, the task
extension does not access tasks.json files directly, and leaves the
work to the preference extension.
resolves tasks configs as preferences #5013

How to test

There are no changes in terms of functionalities. Therefore, to verify this change, we would need to go through the use cases that are already supported by Theia, and check if they are still properly functional. Those use cases include but not limit to:

As a Theia user I am able to

run tasks defined in tasks.json
run detected tasks contributed by Theia plugins
run detected tasks contributed by Theia extensions (Please note, this is broken on master branch and reported in cpp build task does not work #6204)
configure tasks defined in tasks.json
configure detected tasks (Please note, "configuring tasks" by clicking the gear icons in tasks' quick open menu is broken on master branch and reported in Can not configure a task from Terminal => Run Task menu #6212. In order to test this feature we would have to use the top menu bar Terminal -> Configure Tasks ...)
see the recently used tasks be grouped together and show up at the top of the tasks' quick open menu
see the "detected tasks" displayed as "configured tasks", if they are configured / customized in the tasks.json
see the problem matcher items show up in the prompt, if a task that is not associated with any problem matchers is started
- and see the selected problem matcher item written into tasks.json
do all the items described above in both single root and multi root workspace.

akosyakov

Awesome changes! Code looks very good besides minor remarks.

From user perspective: content assist does not work in tasks.json regardless whether I create it in .theia or .vscode folder.

packages/task/src/browser/task-configuration-model.ts

packages/task/src/browser/task-configuration-manager.ts

packages/task/src/browser/task-configurations.ts

akosyakov · 2019-09-27T01:59:32Z

@RomanNikitenko @vince-fugnitto if you do smoke testing it would help too, just to be safe, i.e. with some VS Code extensions. You should be able to define now tasks.json in .vscode or .theia folder and in settings.json under tasks attribute as well.

elaihau · 2019-09-27T03:28:28Z

Thank you for the review Anton !
I addressed the comments, and will test if task configs can be defined in preferences. Never thought of trying with this change :)

akosyakov · 2019-09-27T08:38:46Z

@elaihau more important that it can be loaded from .vscode, from settings.json is probably very rare

akosyakov · 2019-09-27T08:39:36Z

packages/task/src/browser/task-configuration-manager.ts

+                model.onDispose(() => this.models.delete(key));
+                this.models.set(key, model);
+            }
+            this.onDidChangeTaskConfigEmitter.fire({ uri: key, type: FileChangeType.UPDATED });


It should happen only in if branch, if model already exists then we don't need to fire update event

akosyakov

Code changes looks good to me just please have a look at https://github.com/eclipse-theia/theia/pull/6268/files#r328968945

I can define and run tasks now under .vscode as well, content assist is there.

vince-fugnitto

I tested the following functionality in Theia:

Use Case	Status
`run task` without selecting a problem matcher	👍
`run task` selecting 'never ask' (this successfully writes `"problemMatcher": []` in the `tasks.json`	👍
`run task` which has `[]` as a problem matcher (does not prompt me again)	👍
`run task` selecting a problem matcher (this successfully writes the problem matcher in the `tasks.json`	👍
`run task` which has a problem matcher (does not prompt me again)	👍
verify recently used tasks are present in the `run task` quick-open	👍
verify if clearing recently used tasks removes recently used tasks from the quick-open	👍
verify if `configure tasks` works	👍
verify if `run last task` works	👍
verify if `show running tasks` works	👍
`.vscode/tasks.json` has content assist	👍
able to define and run tasks from the `.vscode/tasks.json` (might be an error on my part, to be confirmed)	\
multi-root works	👍

- In current Theia, the task extension is responsible for reading, writing, and watching `tasks.json` files. With this change, the task extension does not access `tasks.json` files directly, and leaves the work to the preference extension. - resolves #5013 Signed-off-by: Liang Huang <[email protected]>

elaihau · 2019-09-28T16:25:59Z

@vince-fugnitto thank you for the review, really apprecited !

I tested the scenario where task configs are defined in .vscode/tasks.json

vince-fugnitto · 2019-09-29T02:23:22Z

@vince-fugnitto thank you for the review, really apprecited !
I tested the scenario where task configs are defined in .vscode/tasks.json

I confirmed that it also works for me to define tasks under .vscode/tasks.json.
I however attempted to have both .vscode and .theia tasks and could not (only sees the .theia tasks). This is likely to be the expected behavior but I was not sure.

elaihau · 2019-09-29T02:30:35Z

I confirmed that it also works for me to define tasks under .vscode/tasks.json.
I however attempted to have both .vscode and .theia tasks and could not (only sees the .theia tasks). This is likely to be the expected behavior but I was not sure.

yep i believe what you described was from Anton's code:

theia/packages/core/src/browser/preferences/preference-configurations.ts

Line 37 in 02c5c1d

/* prefer Theia over VS Code by default */

and I think it's expected.

elaihau requested review from akosyakov, RomanNikitenko and vince-fugnitto September 26, 2019 10:53

elaihau force-pushed the Liang/task_pref branch 3 times, most recently from ec7a447 to 8f66f5a Compare September 26, 2019 22:49

akosyakov added preferences issues related to preferences tasks issues related to the task system labels Sep 27, 2019

akosyakov reviewed Sep 27, 2019

View reviewed changes

elaihau force-pushed the Liang/task_pref branch from 8f66f5a to b477487 Compare September 27, 2019 03:27

akosyakov reviewed Sep 27, 2019

View reviewed changes

akosyakov approved these changes Sep 27, 2019

View reviewed changes

vince-fugnitto reviewed Sep 27, 2019

View reviewed changes

elaihau force-pushed the Liang/task_pref branch from b477487 to 6ec8347 Compare September 28, 2019 16:40

elaihau merged commit b059e2e into master Sep 29, 2019

elaihau deleted the Liang/task_pref branch September 29, 2019 13:37

elaihau mentioned this pull request Apr 23, 2020

[preferences][multi root] Potentially exagerated number of calls to getConfigUri() #7631

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

access tasks configs as preferences #6268

access tasks configs as preferences #6268

elaihau commented Sep 26, 2019 •

edited

Loading

akosyakov left a comment

akosyakov commented Sep 27, 2019 •

edited

Loading

elaihau commented Sep 27, 2019

akosyakov commented Sep 27, 2019

akosyakov Sep 27, 2019 •

edited

Loading

akosyakov left a comment

vince-fugnitto left a comment

elaihau commented Sep 28, 2019

vince-fugnitto commented Sep 29, 2019

elaihau commented Sep 29, 2019

access tasks configs as preferences #6268

access tasks configs as preferences #6268

Conversation

elaihau commented Sep 26, 2019 • edited Loading

How to test

akosyakov left a comment

Choose a reason for hiding this comment

akosyakov commented Sep 27, 2019 • edited Loading

elaihau commented Sep 27, 2019

akosyakov commented Sep 27, 2019

akosyakov Sep 27, 2019 • edited Loading

Choose a reason for hiding this comment

akosyakov left a comment

Choose a reason for hiding this comment

vince-fugnitto left a comment

Choose a reason for hiding this comment

elaihau commented Sep 28, 2019

vince-fugnitto commented Sep 29, 2019

elaihau commented Sep 29, 2019

elaihau commented Sep 26, 2019 •

edited

Loading

akosyakov commented Sep 27, 2019 •

edited

Loading

akosyakov Sep 27, 2019 •

edited

Loading