Enable user-level task configurations #7620

tsmaeder · 2020-04-20T09:45:31Z

Signed-off-by: Thomas Mäder tmader@redhat.com
Fixes: #7446 (Support User Level Tasks)

What it does

This PR adds support for adding tasks in ~/.theia/tasks.json. It does so by extending the support for preference configurations to user-level preferences and accessing that preference scope directly for reading tasks. That code has been refactored a bit to enable this.
Contributed tasks can not be configured as user-level tasks, and only shell and terminal task types work as user-level tasks (same as in VS Code.
A command to "Tasks: Open User Tasks" has been added that opens an editor on the user-level tasks.json

How to test

Go through the various commands starting with "Tasks:..." and make sure they still function properly for folder-level tasks.
Add tasks to ~/.theia/tasks.json and verify they work as expected.

Known problems

Currently, the combined schema for all task types is used in the user tasks.json, whereas only the shell and terminal types should be allowed in that file. I would postpone that fix to keep this PR more manageable.

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

tsmaeder · 2020-04-20T10:53:55Z

Rebased.

elaihau

Thank you for the work.
I played with this change. One thing that I noticed was, with this change, the inputs variables defined in the ~/.theia/tasks.json can be used & overridden in my {workspaceFolder}/.theia/tasks.json, while in vscode it is not the case.

in my opinion it is a better way of handling inputs than vsCode.

One problem that I found was, the command Task: Open User Tasks opens ~/.theia/tasks.json in a new tab even if the same file has been opened.

tsmaeder · 2020-04-21T07:14:50Z

the inputs variables defined in the ~/.theia/tasks.json can be used & overridden in my {workspaceFolder}/.theia/tasks.json, while in vscode it is not the case.

I don't quite understand what you describe here: can you give a concrete example?

... the command Task: Open User Tasks opens ~/.theia/tasks.json in a new tab even if the same file has been opened.

I am using the "userstorage://" URI instead of a file URI. I think that is the correct thing to do abstractionwise and we don't really have a way to resolve that to a "real" URI on the front end. Open to suggestions here.

elaihau · 2020-04-21T20:59:16Z

the inputs variables defined in the ~/.theia/tasks.json can be used & overridden in my {workspaceFolder}/.theia/tasks.json, while in vscode it is not the case.

I don't quite understand what you describe here: can you give a concrete example?

I meant, with the following ~/.theia/tasks.json:

{
    "tasks": [
        {
            "type": "shell",
            "label": "user test",
            "command": "sleep 1 && echo ${input:arg1}"
        }
    ],
    "inputs": [
        {
            "id": "arg1",
            "type": "promptString",
            "default": "test arg 1",
            "description": "dadsf"
        }
    ]
}

I was able to run the task shell: ws test defined in my {workspaceFolder}/.theia/tasks.json:

{
  "tasks": [
    {
      "type": "shell",
      "label": "ws test",
      "command": "sleep 1 && echo ${input:arg1}"
    }
  ]
}

I can even redefine input:arg1 in my workspace to override the one defined in the userstorage.

vscode does not allow it. But i am not against you. i like your solution more.

akosyakov

I did a pass through the code and it looks good to me. VS Code compatibility tests were not changed and still passing. I will do some manual regression testing and check whether we need any records in CHANGELOG. It would be good that someone else tests whether user level task configurations work now.

packages/core/src/browser/preferences/preference-service.ts

akosyakov

I've tested debug launch configuration and could not find regressions.
I think changes also done clean. I am not sure that we need breaking changes. For user preferences adopters actually has to provide a custom implementation of UserStorageService to store them somewhere else. This PR does not change it and still resolves all user configuration files via the user storage service.

Anyway @elaihau @RomanNikitenko It would be helpful if you look over the task extension improvemetns.

elaihau

Maybe the message in the prompt for user tests could be more user-friendly ?

This is my task config:

        {
            "type": "shell",
            "label": "user test",
            "command": "sleep 1 && pwd && echo ${input:arg1}"
        },

I am not sure where the (1) in the screenshot comes from
In a multi root workspace, we also display the root folder as part of the message. So in the message do we want to mention that it is a user test ?

elaihau · 2020-04-29T02:14:24Z

I am unable to configure the user test

elaihau · 2020-04-29T02:39:37Z

Open a multi-root workspace that has 2 root folders, say, rootFolderA and rootFolderB, and define an input variable in ~/.theia/tasks.json

    "inputs": [
        {
            "id": "arg1",
            "type": "promptString",
            "default": "test arg 1",
            "description": "dadsf"
        }
    ]

Add an input variable with the same name in {rootFolderA}/tasks.json

    "inputs": [
        {
            "id": "arg1",
            "type": "promptString",
            "default": "test arg 2",
            "description": "dadsf"
        }
    ]

Add dependent tasks to ~/.theia/tasks.json:

    "tasks": [
        {
            "label": "test arg1",
            "type": "shell",
            "command": "sleep 1 && echo 1 && echo ${input:arg1}"
        },
        {
            "label": "test arg2",
            "type": "shell",
            "command": "sleep 1 && echo 2 && echo ${input:arg1}",
            "dependsOn": ["test arg1"]
        }
    ]

Start user test "test arg2", and observe the default value of input:arg1 - it is test arg2 instead of test arg1. Looks like the input variable defined at the folder level overrides the one defined at the user level. And I don't think it is correct.

tsmaeder · 2020-04-29T06:30:56Z

@elaihau

Looks like the input variable defined at the folder level overrides the one defined at the user level. And I don't think it is correct.

Why do you think it's incorrect and what would the correct behaviour be in your opinion? I just tested this and it seems VS Code behaves the same way. In fact: as soon as I define an "inputs" section in the workspace folder, the inputs from the user level become undefined: it seems VS Code is using the normal preference override mechanism for inputs.

I am unable to configure the user test

True dat! We should just open the user tasks resource at the corresponding place.

I am not sure where the (1) in the screenshot comes from

It seems the scope of the task is used here: the enumeration "TaskScope.User" has the value 1

tsmaeder · 2020-04-29T09:46:44Z

I fixed the source rendering and open user tasks when configuring a user task, as per @elaihau's comments.

elaihau · 2020-04-29T11:36:37Z

Looks like the input variable defined at the folder level overrides the one defined at the user level. And I don't think it is correct.

Why do you think it's incorrect and what would the correct behaviour be in your opinion? I just tested this and it seems VS Code behaves the same way. In fact: as soon as I define an "inputs" section in the workspace folder, the inputs from the user level become undefined: it seems VS Code is using the normal preference override mechanism for inputs.

I tried in vs code one more time, and I DIDN'T see the value of the input variable defined in the root folder.

As you can see from the GIF, in vscode, although {input:arg1} has the default value of "test arg 2" in my root folder, when i ran the user test the default was "test arg 1"

In my opinion, the default value of input variable, if defined in a root folder, should only be used in that folder, not in the user tests.

tsmaeder · 2020-04-30T09:46:01Z

In my opinion, the default value of input variable, if defined in a root folder, should only be used in that folder, not in the user tests.

I don't agree with this one: one could define a global task that uses different inputs depending on the workspace folder. In any case, I don't see it as creating any problems, so I would move that we just leave that as it is. Did you check how this works in VS Code?

elaihau · 2020-04-30T10:59:33Z

Did you check how this works in VS Code?

Yes I did check how it works in VS Code.

Please see my comment #7620 (comment)

The GIF in that comment was also made from VS Code.

one could define a global task that uses different inputs depending on the workspace folder.

I see where you come from.

But think about this scenario: In a multi root workspace composed of rootA and rootB

input:arg1 has the default value of 1
the default value of input:arg1 = A in root A
the default value of input:arg1 = B in root B
we run the user task that references input:arg1

Which deault value we should expect input:arg1 to take? in this case, there are ways for us to determine the root folder that the user test fetches the default value from. But I don't see it be properly managed in this change.

tsmaeder · 2020-04-30T12:02:59Z

But I don't see it be properly managed in this change.

When an active root folder can be determined (open editor, basically) the context for variable evaluation becomes that root folder. When no root folder can be determined (do a file->open of a file outside any workspace root), the first workspace folder is used.

elaihau · 2020-04-30T12:48:06Z

But I don't see it be properly managed in this change.

When an active root folder can be determined (open editor, basically) the context for variable evaluation becomes that root folder. When no root folder can be determined (do a file->open of a file outside any workspace root), the first workspace folder is used.

If we don't want to follow the VS code behavior
yep it makes sense to me whereas i don't see it work as per your description with this change. as you can see from the GIF below (made from Theia with this change), although my editor opens a file in the 2nd root folder where input:arg1 has the default value of test arg 3, the user test uses test arg 1.

maybe i missed something

If we do want to follow what VS code does
The way that input variables work in this change does not follow vs code either

tsmaeder · 2020-04-30T13:03:15Z

@elaihau I'm really having trouble with your animated gifs: they proceed really fast and I can't install a plugin to stop them on Firefox.

tsmaeder · 2020-04-30T13:11:06Z

"Steps to reproduce" are much appreciated, unless it's something visual.

elaihau · 2020-04-30T13:18:24Z

"Steps to reproduce" are much appreciated, unless it's something visual.

sure here they are

open a workspace composed of rootA and rootB
define user tasks and a input var

    "tasks": [
        {
            "label": "test arg1",
            "type": "shell",
            "command": "sleep 1 && echo 1 && echo ${input:arg1}"
        },
        {
            "label": "test arg2",
            "type": "shell",
            "command": "sleep 1 && echo 2 && echo ${input:arg1}",
            "dependsOn": ["test arg1"]
        }
    ],
    "inputs": [
        {
            "id": "arg1",
            "type": "promptString",
            "default": "test arg 1",
            "description": "dadsf"
        }
    ]

redefine input var arg1 in rootB

    "inputs": [
        {
            "id": "arg1",
            "type": "promptString",
            "default": "test arg 3",
            "description": "dadsf"
        }
    ]

while the editor still opens tasks.json from rootB, use ctrl shift p -> run task to run "test arg2" defined in Step 2, and check which default value the user task uses.

In my env, the user task uses test arg 1. and as per your description, if we don't want to follow what vs code does, test arg 3 should be used, because my editor opens a file in rootB. Please correct me if i am wrong. thanks !

tsmaeder · 2020-04-30T13:24:00Z

sure here they are
thx, appreciate it.

Let me check your case: it's supposed to work, don't know why not.

VS Code does not seem to inherit any inputs from the user level: it refuses to run a folder-level task unless the input var is defined in the same folder. I suspect making this the same as VS code would be hard, since we'd have to special-case it, like the task configurations, I suspect.

akosyakov · 2020-05-01T07:54:28Z

VS Code does not seem to inherit any inputs from the user level: it refuses to run a folder-level task unless the input var is defined in the same folder. I suspect making this the same as VS code would be hard, since we'd have to special-case it, like the task configurations, I suspect.

@tsmaeder Is it important right now for your use case?
@elaihau If not maybe filing an issue will be enough for now and we can tackle it with another PR?

elaihau · 2020-05-03T00:52:59Z

@tsmaeder Is it important right now for your use case?
@elaihau If not maybe filing an issue will be enough for now and we can tackle it with another PR?

i am fine with it being tackled as a separate issue.

tsmaeder · 2020-05-08T09:58:16Z

Seems the "context key" always decides that the "resource" is the first workspace folder. That seems to happen inside monaco. 🤷

tsmaeder · 2020-05-08T12:08:18Z

Actually, "it's me honey, not you"....I'm setting the resolution context to the wrong place :-(

tsmaeder · 2020-05-08T13:20:12Z

@elaihau your case should be addressed in the latest commit.

elaihau

Tested again. Looks really good to me. Thank you for the work !

Signed-off-by: Thomas Mäder <tmader@redhat.com>

akosyakov · 2020-05-11T08:47:51Z

Looks good. Looked through the code again and smoked test debugging from different launch configurations.

tsmaeder · 2020-05-11T11:01:20Z

I've squashed the commits and would merge later today.

tsmaeder requested review from vince-fugnitto, akosyakov and elaihau April 20, 2020 09:45

tsmaeder force-pushed the 7446_user_level_tasks branch from 75b4063 to be4f301 Compare April 20, 2020 10:53

vince-fugnitto added the tasks issues related to the task system label Apr 20, 2020

elaihau reviewed Apr 21, 2020

View reviewed changes

akosyakov reviewed Apr 22, 2020

View reviewed changes

packages/core/src/browser/preferences/preference-service.ts Outdated Show resolved Hide resolved

akosyakov approved these changes Apr 24, 2020

View reviewed changes

akosyakov added preferences issues related to preferences debug issues that related to debug functionality labels Apr 24, 2020

elaihau reviewed Apr 29, 2020

View reviewed changes

tsmaeder mentioned this pull request Apr 29, 2020

Support Che tasks for multi-root workspaces eclipse-che/che#15540

Closed

tsmaeder force-pushed the 7446_user_level_tasks branch from 6a2356d to 0944817 Compare April 29, 2020 11:43

tsmaeder force-pushed the 7446_user_level_tasks branch from ebb3cb7 to 3fcd68b Compare May 8, 2020 13:19

tsmaeder mentioned this pull request May 8, 2020

15540 user level tasks eclipse-che/che-theia#739

Merged

elaihau approved these changes May 9, 2020

View reviewed changes

Enable user-level task configurations

415d602

Signed-off-by: Thomas Mäder <tmader@redhat.com>

tsmaeder force-pushed the 7446_user_level_tasks branch from 3fcd68b to 415d602 Compare May 11, 2020 06:50

tsmaeder mentioned this pull request May 12, 2020

Fix compilation for theia changes eclipse-che/che-theia#743

Closed

tsmaeder merged commit 98782b6 into eclipse-theia:master May 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable user-level task configurations #7620

Enable user-level task configurations #7620

tsmaeder commented Apr 20, 2020 •

edited by vince-fugnitto

Loading

tsmaeder commented Apr 20, 2020

elaihau left a comment •

edited

Loading

tsmaeder commented Apr 21, 2020

elaihau commented Apr 21, 2020

akosyakov left a comment

akosyakov left a comment

elaihau left a comment •

edited

Loading

elaihau commented Apr 29, 2020

elaihau commented Apr 29, 2020

tsmaeder commented Apr 29, 2020 •

edited

Loading

tsmaeder commented Apr 29, 2020

elaihau commented Apr 29, 2020 •

edited

Loading

tsmaeder commented Apr 30, 2020

elaihau commented Apr 30, 2020 •

edited

Loading

tsmaeder commented Apr 30, 2020

elaihau commented Apr 30, 2020 •

edited

Loading

tsmaeder commented Apr 30, 2020

tsmaeder commented Apr 30, 2020

elaihau commented Apr 30, 2020

tsmaeder commented Apr 30, 2020 •

edited

Loading

akosyakov commented May 1, 2020

elaihau commented May 3, 2020

tsmaeder commented May 8, 2020

tsmaeder commented May 8, 2020

tsmaeder commented May 8, 2020

elaihau left a comment

akosyakov commented May 11, 2020

tsmaeder commented May 11, 2020

Enable user-level task configurations #7620

Enable user-level task configurations #7620

Conversation

tsmaeder commented Apr 20, 2020 • edited by vince-fugnitto Loading

What it does

How to test

Known problems

Review checklist

Reminder for reviewers

tsmaeder commented Apr 20, 2020

elaihau left a comment • edited Loading

Choose a reason for hiding this comment

tsmaeder commented Apr 21, 2020

elaihau commented Apr 21, 2020

akosyakov left a comment

Choose a reason for hiding this comment

akosyakov left a comment

Choose a reason for hiding this comment

elaihau left a comment • edited Loading

Choose a reason for hiding this comment

elaihau commented Apr 29, 2020

elaihau commented Apr 29, 2020

tsmaeder commented Apr 29, 2020 • edited Loading

tsmaeder commented Apr 29, 2020

elaihau commented Apr 29, 2020 • edited Loading

tsmaeder commented Apr 30, 2020

elaihau commented Apr 30, 2020 • edited Loading

tsmaeder commented Apr 30, 2020

elaihau commented Apr 30, 2020 • edited Loading

tsmaeder commented Apr 30, 2020

tsmaeder commented Apr 30, 2020

elaihau commented Apr 30, 2020

tsmaeder commented Apr 30, 2020 • edited Loading

akosyakov commented May 1, 2020

elaihau commented May 3, 2020

tsmaeder commented May 8, 2020

tsmaeder commented May 8, 2020

tsmaeder commented May 8, 2020

elaihau left a comment

Choose a reason for hiding this comment

akosyakov commented May 11, 2020

tsmaeder commented May 11, 2020

tsmaeder commented Apr 20, 2020 •

edited by vince-fugnitto

Loading

elaihau left a comment •

edited

Loading

elaihau left a comment •

edited

Loading

tsmaeder commented Apr 29, 2020 •

edited

Loading

elaihau commented Apr 29, 2020 •

edited

Loading

elaihau commented Apr 30, 2020 •

edited

Loading

elaihau commented Apr 30, 2020 •

edited

Loading

tsmaeder commented Apr 30, 2020 •

edited

Loading