Add `task#runOptions` to API. #11759

sgraband · 2022-10-12T14:02:57Z

What it does

Additionally added functionality to support reevaluateOnRerun. If this flag is set to false no variables will be resolved again. Therefore lastTask can now be re-triggered without resolving all variables. For this the lastTask object was updated to contain all resolved information.

Contributed on behalf of STMicroelectronics

Resolves #11132

How to test

To test you can use the follwing tasks.json:

{
  "version": "2.0.0",
  "tasks": [
    {
      "label": "Node Command",
      "type": "shell",
      "command": "node",
      "args": [
        "${input:node command}"
      ],
      "isBackground": true,
      "runOptions": {
        "reevaluateOnRerun": false
      }
    },
  ],
  "inputs": [
    {
      "id": "node command",
      "description": "node ${input:node command}",
      "default": "--version",
      "type": "promptString"
    },
  ]
}

When you run the node command you will be asked, which command you want to run. If you enter --version for example and then rerun the last task. You will not be asked and node --version is automatically executed.
When you change reevaluateOnRerun to true, run and rerun the task, you will need to reenter the command every time.

To test the API you can use this extension. It provides a task (custombuildscript: 32) with reevaluateOnRerun: true. When you run this task and debug, for example at task-service.ts#808 the task object should have the correct runOptions set.

Review checklist

As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

As a reviewer, I agree to behave in accordance with the review guidelines

colin-grant-work · 2022-10-13T22:34:57Z

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

            return;
        }
-        const { source, taskLabel, scope } = this.lastTask;
-        return this.run(token, source, taskLabel, scope);
+        if (!this.lastTask.resolvedTask.runOptions?.reevaluateOnRerun) {


If this flag is set to false no variables will be resolved again. Therefore lastTask can now be re-triggered without resolving all variables.

It appears that reevaluateOnRerun = false is being treated as the default. That's a change in the expectations for internal tasks, which in the past would always have been resolved when rerun. Is there any value in treating internal and external tasks differently in this respect?

Yes i was wondering this as well. I would find it odd, if the default behavior is reevaluateOnRerun = true as this would mean, that we need to interpret missing options as true. I am not sure, how internal tasks are handled, but would it be a option to add this runOption to all internal tasks? This would keep the current behavior and explicitly explain for each task how it should be handled. WDYT?

@colin-grant-work by "internal", do you mean tasks defined in tasks.json or contributed by non-plugin API? If we need to treat tasks different, I would much prefer putting the difference where we create the tasks (for example by setting a flag in the contructor) and not where we process the tasks. Locality of reference for teh win!

Re: tasks.json, we should likely follow what VSCode does, though there's scope for making our own choice there. Mostly I mean other tasks created internally, but I'm not very familiar with the behavior or UI of 'rerun' tasks (as opposed to running the same task again, but not as a rerun), so I'm not sure how disruptive it would be to change from resolving to not resolving variables in the task description.

One solution might be to modify the plugin system to supply false by default for tasks originating from plugins, and then have a flag in the task service for whether to use false or true as the default value there so that application implementers can easily toggle it if they have a use case where true is appropriate. On the other hand, it sounds like @tsmaeder favors something on the task description itself (in which case maybe setting false in the plugin system and using true as a default otherwise would be the best course?). In any case, I don't know if it really warrants too much concern.

I thought about this some more: what I really would like to avoid is a "if tasksource = plugin, do x, else do y". I'm also not a fan of setting variables that are client API in order to simulate the "right" default behaviour: if anything, we should set a flag : task.defaultRerunOnReevaluate= false that is not part of the API in the plugin task provider implementation and default to true in clients to preserve the current behaviour.
But t.b.h, I would just make the change to the VS Code behavior: it's a breaking change, and while we try not to frivolously break API, we also should not unduly complicate the system to keep the API stable.
My vote is: put the code in as it is and make a note of it in the "breaking changes". Clients can trivially update their code if they rely on the behaviour. @colin-grant-work can you live with that?

I'm fine with this approach.

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

packages/plugin-ext/src/plugin/types-impl.ts

packages/plugin-ext/src/plugin/type-converters.ts

tsmaeder · 2022-10-14T09:30:32Z

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

            return;
        }
-        const { source, taskLabel, scope } = this.lastTask;
-        return this.run(token, source, taskLabel, scope);
+        if (!this.lastTask.resolvedTask.runOptions?.reevaluateOnRerun) {


@colin-grant-work by "internal", do you mean tasks defined in tasks.json or contributed by non-plugin API? If we need to treat tasks different, I would much prefer putting the difference where we create the tasks (for example by setting a flag in the contructor) and not where we process the tasks. Locality of reference for teh win!

colin-grant-work · 2022-10-31T13:08:15Z

CHANGELOG.md

@@ -8,6 +8,8 @@

 - [plugin] added support for the `InlineValues` feature [#11729](https://github.com/eclipse-theia/theia/pull/11729) - Contributed on behalf of STMicroelectronics
 - [plugin] Added support for `resolveTreeItem` of `TreeDataProvider` [#11708](https://github.com/eclipse-theia/theia/pull/11708) - Contributed on behalf of STMicroelectronics
+- [plugin] added `Task#runOptions` field and `RunOptions` interface [#11759](https://github.com/eclipse-theia/theia/pull/11759) - Contributed on behalf of STMicroelectronics
+- [tasks] added support for `reevaluateOnRerun` run option [#11759](https://github.com/eclipse-theia/theia/pull/11759) - Contributed on behalf of STMicroelectronics


As suggested by @tsmaeder, please add a note about the change in behavior for tasks in the breaking changes area.

Done. Could you recheck?

Additionally added functionality to support `reevaluateOnRerun`. If this flag is set to false no variables will be resolved again. Therefore lastTask can now be re-triggered without resolving all variables. For this the lastTask object was updated to contain all resolved information. Contributed on behalf of STMicroelectronics Resolves eclipse-theia#11132

colin-grant-work

The behavior looks good, and adopters should be adequately warned re: any effect on their current setup.

sgraband force-pushed the task#runOptions branch from a627f16 to 03fd853 Compare October 12, 2022 14:37

vince-fugnitto added tasks issues related to the task system vscode issues related to VSCode compatibility labels Oct 12, 2022

JonasHelming requested a review from tsmaeder October 13, 2022 14:19

colin-grant-work reviewed Oct 13, 2022

View reviewed changes

packages/task/src/browser/task-service.ts Outdated Show resolved Hide resolved

tsmaeder reviewed Oct 14, 2022

View reviewed changes

sgraband force-pushed the task#runOptions branch from 03fd853 to 8c57959 Compare October 17, 2022 07:16

sgraband requested a review from colin-grant-work October 18, 2022 12:20

colin-grant-work reviewed Oct 31, 2022

View reviewed changes

sgraband added 2 commits November 2, 2022 09:00

Address review feedback

fd36854

sgraband force-pushed the task#runOptions branch from 8c57959 to fd36854 Compare November 2, 2022 08:00

sgraband requested a review from colin-grant-work November 2, 2022 08:02

colin-grant-work approved these changes Nov 2, 2022

View reviewed changes

colin-grant-work merged commit 385860d into eclipse-theia:master Nov 2, 2022

This was referenced Nov 16, 2022

Add Task#runOptions eclipsesource/theia#47

Closed

Add task#runOptions to API. eclipsesource/theia#52

Closed

vince-fugnitto added this to the 1.32.0 milestone Nov 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `task#runOptions` to API. #11759

Add `task#runOptions` to API. #11759

sgraband commented Oct 12, 2022 •

edited by vince-fugnitto

Loading

colin-grant-work Oct 13, 2022 •

edited

Loading

sgraband Oct 14, 2022

tsmaeder Oct 14, 2022

colin-grant-work Oct 18, 2022

tsmaeder Oct 21, 2022 •

edited

Loading

colin-grant-work Oct 28, 2022

tsmaeder Oct 14, 2022

colin-grant-work Oct 31, 2022

sgraband Nov 2, 2022

colin-grant-work left a comment

Add task#runOptions to API. #11759

Add task#runOptions to API. #11759

Conversation

sgraband commented Oct 12, 2022 • edited by vince-fugnitto Loading

What it does

How to test

Review checklist

Reminder for reviewers

colin-grant-work Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

sgraband Oct 14, 2022

Choose a reason for hiding this comment

tsmaeder Oct 14, 2022

Choose a reason for hiding this comment

colin-grant-work Oct 18, 2022

Choose a reason for hiding this comment

tsmaeder Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

colin-grant-work Oct 28, 2022

Choose a reason for hiding this comment

tsmaeder Oct 14, 2022

Choose a reason for hiding this comment

colin-grant-work Oct 31, 2022

Choose a reason for hiding this comment

sgraband Nov 2, 2022

Choose a reason for hiding this comment

colin-grant-work left a comment

Choose a reason for hiding this comment

Add `task#runOptions` to API. #11759

Add `task#runOptions` to API. #11759

sgraband commented Oct 12, 2022 •

edited by vince-fugnitto

Loading

colin-grant-work Oct 13, 2022 •

edited

Loading

tsmaeder Oct 21, 2022 •

edited

Loading