Skip to content
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

users shouldn't have the flexibility of customizing all properties of a detected task #5679

Closed
elaihau opened this issue Jul 10, 2019 · 10 comments · Fixed by #5777
Closed

users shouldn't have the flexibility of customizing all properties of a detected task #5679

elaihau opened this issue Jul 10, 2019 · 10 comments · Fixed by #5777
Assignees
Labels
tasks issues related to the task system

Comments

@elaihau
Copy link
Contributor

elaihau commented Jul 10, 2019

Description

In theia, when customizing a detected task, users could potentially change any property of the task:

Peek 2019-07-09 23-23

In vscode, users are only allowed to customize 3 things:

export interface CustomizationProperties {
	group?: string | { kind?: string; isDefault?: boolean; };
	problemMatcher?: string | string[];
	isBackground?: boolean;
}

Peek 2019-07-09 23-27

We should align this behavior with vsCode. Conceptually, being able to customize anything is beyond "customization" - it is more like "using the existing task as a template to create a new task".

Reproduction Steps

1, Open the list of tasks which includes at least one detected task
2, Click the gear icon right hand side of the detected task to customize it
3, Check what Theia adds to .theia/tasks.json

@elaihau elaihau added the tasks issues related to the task system label Jul 10, 2019
@elaihau
Copy link
Contributor Author

elaihau commented Jul 10, 2019

@RomanNikitenko @azatsarynnyy
I noticed that most part of the task customization was implemented by Redhat developers. Is there any reason that you would like users to be able to customize all properties of a task? If not, I will make changes to make Theia follow vs code

@elaihau elaihau changed the title users shouldn't have the flexibility of customizing any property of auto-detected tasks users shouldn't have the flexibility of customizing all properties of a detected task Jul 10, 2019
@RomanNikitenko
Copy link
Contributor

RomanNikitenko commented Jul 10, 2019

@elaihau
Hello!
Thank you for the information!

Our tasks contain fields with info about command, working directory, container where a task will be executed and so on. It's really important for us to provide the flexibility of customizing a configuration!

On the other hand I understand that at the moment the customizing of detected tasks in Theia differs from VS Code behavior and an configuration after customizing contain extra fields.

Could you provide more info about the way how you are going to achieve the alignment?
Will it influence on detected tasks only and the process of configuring them?

To help me understand how we can adapt our code, let us suppose, at the start of a workspace our tasks with all required fields are copied to tasks.json file without user interaction.
Will Theia take a configuration as is (with all fields) for running from tasks.json file? Or you are going to take only some fields from config file and add other fields before running.

Thanks in advance!

@elaihau
Copy link
Contributor Author

elaihau commented Jul 10, 2019

Could you provide more info about the way how you are going to achieve the alignment?
Will it influence on detected tasks only and the process of configuring them?

To make myself clear, I want to define what "customize" means to me first.
Regardless of what VS Code / Theia UI displays, in my opinion, "Customize" a task means adding an object to tasks.json to overwrite / override what an existing task has.

With my definion of "customize" in mind, only detected tasks can be customized.
Although VS Code & Theia display the clickable "gear icons" on the right hand side of configured tasks, users never get the chance to "customize" them - they are just using the clickable gear icons to go to tasks.json where they can update configured tasks (I used the word "update", not "customize", as users don't add new objects to the json).

Therefore, I don't think the process of customizing / updating / adding / removing configured tasks will be changed.

To help me understand how we can adapt our code, let us suppose, at the start of a workspace our tasks with all required fields are copied to tasks.json file without user interaction.
Will Theia take a configuration as is (with all fields) for running from tasks.json file? Or you are going to take only some fields from config file and add other fields before running.

The code that runs configured tasks will NOT be touched at all.

Running detected tasks, if aligned with VS Code, should work this way in Theia:
1, Task Service reads everything from tasks.json,
2, uses the task definitions contributed by extensions and plug-ins to tell which tasks are configured tasks, and which ones are detected tasks,
3, takes group, isBackground, and problemMatcher from the config, applies the 3 properties to the detected task, and
4, runs the detected task selected by the user.

The four-step process described above is just a proposal. I understand there could be a reason behind "offering flexibility to customize all task properties". And it is why I would like to hear opinions & suggestions from the community before writing code :)

@RomanNikitenko

@elaihau
Copy link
Contributor Author

elaihau commented Jul 10, 2019

Roman, if possible, could you please give me an example where users need to customize properties such as task.options.cwd or task._id in a detected task? I don't know much about Che so I might have missed a lot of use cases we should support.

@RomanNikitenko
Copy link
Contributor

RomanNikitenko commented Jul 12, 2019

@elaihau
thank you very much for the detailed response!

Roman, if possible, could you please give me an example where users need to customize properties such as task.options.cwd or task._id in a detected task? I don't know much about Che so I might have missed a lot of use cases we should support.

I think users don't have needs to customize task._id field and this is extra field for a configuration in tasks.json file.
About task.options.cwd - it's possible to change a working directory for a che task, but we consider the possibility to move che tasks to tasks.json file, so I hope detected tasks can be aligned with VS Code painless for che project.

With my definion of "customize" in mind, only detected tasks can be customized.

Yes, I understand and agree with you. But in VS Code terms what's the best way for user to add a new task? Not override some task, just add a new task.

We provided our tasks using plugin API (so, as detected) and had some issues (related to way of determining is a task configured or detected). Now we consider tasks.json file as one place where user can find all che tasks, edit them or add a new task. Is it OK from your point of view?

As an example of che task:

           {
                "type": "che",
                "label": "run che-theia + container-plugin",
                "command": "kill `cat /tmp/node_theiadev.pid`; rm /default-theia-plugins/eclipse_che_theia_containers_plugin.theia; mkdir -p /tmp/theiadev_projects && export CHE_PROJECTS_ROOT=/tmp/theiadev_projects && export HOSTED_PLUGIN=/projects/che-theia/plugins/containers-plugin/ && node src-gen/backend/main.js /tmp/theiadev_projects --hostname=0.0.0.0 --port=3130 & echo $!> /tmp/node_theiadev.pid ; wait `cat /tmp/node_theiadev.pid`\n",
                "target": {
                    "workingDir": "/projects/theia/production",
                    "containerName": "che-dev"
                }
            }

You can see that che task doesn't have a lot of fields, but can have quite complicated command, so it's important to provide ability for user to edit it, change some args and so on. Also user can change container or working directory where he wants to run it .

After align with VS Code behavior - what will happen if user runs a configuration from tasks.json, but associated detected task is not found for this configuration? Will the configuration from tasks.json deliver as it is for corresponding runner?

We register own runner for che tasks, so we have ability to handle them correctly. But it's very important for us to deliver configuration as it is from tasks.json file to our runner.

Thank you!

@elaihau
Copy link
Contributor Author

elaihau commented Jul 13, 2019

After align with VS Code behavior - what will happen if user runs a configuration from tasks.json, but associated detected task is not found for this configuration? Will the configuration from tasks.json deliver as it is for corresponding runner?

@RomanNikitenko
I tested this scenario in VS Code. If in tasks.json you have a config, and no detected task can be found for it, that config won't be displayed in the list of quick-open-task, and therefore, users won't get the chance to run it.

We register own runner for che tasks, so we have ability to handle them correctly. But it's very important for us to deliver configuration as it is from tasks.json file to our runner.

maybe what I can do is,

  • making what "configure task" does in VS Code the default behavior of Theia, meanwhile,
  • allowing the Theia user to override any property of a dectected task.

Take your che task for example,

           {
                "type": "che",
                "label": "run che-theia + container-plugin",
                "command": "kill `cat /tmp/node_theiadev.pid`; rm /default-theia-plugins/eclipse_che_theia_containers_plugin.theia; mkdir -p /tmp/theiadev_projects && export CHE_PROJECTS_ROOT=/tmp/theiadev_projects && export HOSTED_PLUGIN=/projects/che-theia/plugins/containers-plugin/ && node src-gen/backend/main.js /tmp/theiadev_projects --hostname=0.0.0.0 --port=3130 & echo $!> /tmp/node_theiadev.pid ; wait `cat /tmp/node_theiadev.pid`\n",
                "target": {
                    "workingDir": "/projects/theia/production",
                    "containerName": "che-dev"
                }
            }

It is contributed by a Theia plugin so it will be displayed as a detected task at the beginning.
Assuming that the definition of this task is

{
        "type": "che",
        "required": [
          "target"
        ],
        "properties": {
          "target": {
            "type": "object"
          }
        }
      }

when users click the "gear icon" to customize this task, they would see the following json added to tasks.json (default behavior):

{
                "type": "che", // "type" and "target" are two required properties. only required properties are written into tasks.json
                "target": { 
                    "workingDir": "/projects/theia/production",
                    "containerName": "che-dev"
                },
                "problemMatcher": []
}

if the user wants, s/he can add "label" and / or "command" into the object to override the original value(s).

Would this work for you?

@RomanNikitenko
Copy link
Contributor

@elaihau

I tested this scenario in VS Code. If in tasks.json you have a config, and no detected task can be found for it, that config won't be displayed in the list of quick-open-task, and therefore, users won't get the chance to run it.

I tried to do something similar to described case here.
So, I:

  • opened theia project using VS Code
  • copied file to the root of the project
  • added the configuration to tasks.json file
       {
            "label": "Run my test task",
            "type": "shell",
            "command": "./task-long-running",
            "group": "test"            
        }
  • can see it as configured from Terminal => Run Task menu
  • can run it

Please see the short video: https://youtu.be/8l2UvtR9xGE

@elaihau
Copy link
Contributor Author

elaihau commented Jul 16, 2019

@elaihau

I tested this scenario in VS Code. If in tasks.json you have a config, and no detected task can be found for it, that config won't be displayed in the list of quick-open-task, and therefore, users won't get the chance to run it.

I tried to do something similar to described case here.
So, I:

  • opened theia project using VS Code
  • copied file to the root of the project
  • added the configuration to tasks.json file
       {
            "label": "Run my test task",
            "type": "shell",
            "command": "./task-long-running",
            "group": "test"            
        }
  • can see it as configured from Terminal => Run Task menu
  • can run it

Please see the short video: https://youtu.be/8l2UvtR9xGE

@RomanNikitenko
Sorry for the confusion. Let me change a few words and put it this way:

I tested this scenario in VS Code. If in tasks.json you have a customization config (https://code.visualstudio.com/docs/editor/tasks#_customizing-autodetected-tasks), and no detected task can be found for it, that customization config won't be displayed in the list of quick-open-task, and therefore, users won't get the chance to run the detected task associated with.

The task you added to the json file

       {
            "label": "Run my test task",
            "type": "shell",
            "command": "./task-long-running",
            "group": "test"            
        }

is a configured task, not a customization config, so you can run it without having any issues.

@RomanNikitenko
Copy link
Contributor

@elaihau
thank you for clarification!

So, if I understand correctly, alignment with VS Code behavior will have no influence on configured tasks.
We can add our configurations to a task.json file and the configurations will be delivered as it is from tasks.json file to our runner.
Right?

@elaihau
Copy link
Contributor Author

elaihau commented Jul 17, 2019

@RomanNikitenko exactly ! no impact on the configured tasks.

@elaihau elaihau self-assigned this Jul 22, 2019
elaihau pushed a commit that referenced this issue Jul 23, 2019
- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)
- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
elaihau pushed a commit that referenced this issue Jul 24, 2019
- edit the right task.json when clicking "configure task" in multi-root
workspace (fixed #4919)

- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)

- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

- The detected tasks, once customized, should be displayed as configured tasks in the quick open. (fixed #5747)

- The same task shouldn’t have more than one customization. Otherwise it would cause ambiguities and duplication in tasks.json (fixed #5719)

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
elaihau pushed a commit that referenced this issue Jul 24, 2019
- edit the right task.json when clicking "configure task" in multi-root
workspace (fixed #4919)

- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)

- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

- The detected tasks, once customized, should be displayed as configured tasks in the quick open. (fixed #5747)

- The same task shouldn’t have more than one customization. Otherwise it would cause ambiguities and duplication in tasks.json (fixed #5719)

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
elaihau pushed a commit that referenced this issue Jul 27, 2019
- edit the right task.json when clicking "configure task" in multi-root
workspace (fixed #4919)

- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)

- allow users to override any task properties other than the ones used in the task definition.

- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

- The detected tasks, once customized, should be displayed as configured tasks in the quick open. (fixed #5747)

- The same task shouldn’t have more than one customization. Otherwise it would cause ambiguities and duplication in tasks.json (fixed #5719)

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
elaihau pushed a commit that referenced this issue Jul 28, 2019
- edit the right task.json when clicking "configure task" in multi-root
workspace (fixed #4919)

- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)

- allow users to override any task properties other than the ones used in the task definition.

- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

- The detected tasks, once customized, should be displayed as configured tasks in the quick open. (fixed #5747)

- The same task shouldn’t have more than one customization. Otherwise it would cause ambiguities and duplication in tasks.json (fixed #5719)

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
elaihau pushed a commit that referenced this issue Jul 28, 2019
- edit the right task.json when clicking "configure task" in multi-root
workspace (fixed #4919)

- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)

- allow users to override any task properties other than `type`, and those that are used to define the in its task definition.

- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

- The detected tasks, once customized, should be displayed as configured tasks in the quick open. (fixed #5747)

- The same task shouldn’t have more than one customization. Otherwise it would cause ambiguities and duplication in tasks.json (fixed #5719)

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
elaihau pushed a commit that referenced this issue Aug 1, 2019
- edit the right task.json when clicking "configure task" in multi-root
workspace (fixed #4919)

- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)

- allow users to override any task properties other than `type`, and those that are used to define the in its task definition.

- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

- The detected tasks, once customized, should be displayed as configured tasks in the quick open. (fixed #5747)

- The same task shouldn’t have more than one customization. Otherwise it would cause ambiguities and duplication in tasks.json (fixed #5719)

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
elaihau pushed a commit that referenced this issue Aug 1, 2019
- edit the right task.json when clicking "configure task" in multi-root
workspace (fixed #4919)

- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)

- allow users to override any task properties other than `type`, and those that are used to define the in its task definition.

- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

- The detected tasks, once customized, should be displayed as configured tasks in the quick open. (fixed #5747)

- The same task shouldn’t have more than one customization. Otherwise it would cause ambiguities and duplication in tasks.json (fixed #5719)

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tasks issues related to the task system
Projects
None yet
2 participants