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

Support commands macro in Theia tasks #8974

Closed
5 tasks done
vparfonov opened this issue Mar 1, 2018 · 17 comments
Closed
5 tasks done

Support commands macro in Theia tasks #8974

vparfonov opened this issue Mar 1, 2018 · 17 comments
Assignees
Labels
kind/enhancement A feature request - must adhere to the feature request template.

Comments

@vparfonov
Copy link
Contributor

vparfonov commented Mar 1, 2018

@vparfonov vparfonov added kind/enhancement A feature request - must adhere to the feature request template. team/ide sprint/next labels Mar 1, 2018
@azatsarynnyy
Copy link
Member

I've created the issue eclipse-theia/theia#1561 in Theia upstream to add support for variable substitution mechanism.

@azatsarynnyy
Copy link
Member

upd:
I finished with the Variable Resolver Extension and merged it into the master of Theia upstream eclipse-theia/theia#1581.
Today, I gonna to start adding support for variables in tasks.json - eclipse-theia/theia#1596.

@azatsarynnyy
Copy link
Member

upd:
Done (macros support):

  • Theia provides service to perform Variable (formerly macros) substitution in strings. Currently, that service is used by Theia itself to resolve the Variables in Tasks. We'll use that service also for resolving Che Preview URLs.
  • Theia exposes Variable API that is used by extensions to contribute the custom Variables. We'll use it for plugging our Variables which are resolved to Che Workspace related info.

Before I continue to work on this task I'd like to clarify what the next steps should be since there's no specification for that.
Currently, Theia Task extension doesn't provide any contribution points to extend its functionality. To add target and preview URL support we need Task API in Theia which would allow us to do the undermentioned tasks:

  • Define our own Task type (Che) in order to associate the additional properties with a Task, e.g. target or preview URL. Currently, the format of Task in Theia is predefined and cannot be extended.
  • Plug our own way of running/killing the Tasks of Che type - executing it in Che machine (AFAIK by calling some remote service). Currently, Theia Task extension spawns the processes on the Theia backend (optionally as PTY, i.e. showing output in a terminal widget).
  • Show Che task's output on UI. Existing Theia's terminal widget could be used for that but it requires some enhancements.
    Since the widget uses xterm.js it could be connected to any endpoint for listening to the output.
    But the widget is designed to work with the processes running on the Theia backend, i.e. on closing, it tries to kill a related process on Theia server etc.
  • Plug some task provider that would add the tasks programmatically to the system instead of having it in the tasks.json. As far as I understand there are several places Che commands may come from, e.g. workspace config, feature/service config etc.

@vparfonov @slemeur do you have some thoughts about it? If you have no objections, I'll prepare a proposal for Task API into Theia upstream.

@vparfonov
Copy link
Contributor Author

sounds good to me

@slemeur
Copy link
Contributor

slemeur commented Apr 10, 2018

Excellent analyze @azatsarynnyy !
Agree with your goal of proposing Tasks API in upstream.

@azatsarynnyy
Copy link
Member

issue status update:
Variables (formerly macros) - done:

  • Theia Variable API has been introduced that allows contributing the custom variables
  • variable resolution works in Che Tasks for command and preview URL fields

Target - done:

  • one can specify a workspace and machine for executing a Che Task

Preview URL - almost done:

  • preview URL can be defined in a Che Task
  • one can choose a preview URL of one of the running Tasks to navigate to

In order to close the subtask for preview URL support, I have to implement dynamic registration of variables for Che machine's servers, e.g. ${server.tomcat}.
Also, I need to finalize Task API in the Theia upstream in order to propose a PR for a review.

Missing features which depend on machine-exec server functionality:

  • it's impossible to kill a running Che Task since the machine-exec server doesn't support killing an exec;
  • Che Task is considered as never-ending since the machine-exec server doesn't provide a notification when an exec is finished.

@azatsarynnyy
Copy link
Member

Added the subtask to add support for showing a task's output.
I'll attach the demos of the implemented features a bit later.

@azatsarynnyy
Copy link
Member

azatsarynnyy commented May 29, 2018

Command Target:
target

It can be improved later:

  • workspace ID may be optional. Use the current workspace's ID by default
  • machine name may be optional. Use quick open widget (machine picker) to choose a target machine

Preview URL:
previewurl

@slemeur
Copy link
Contributor

slemeur commented May 29, 2018

Thanks for attaching the recording @azatsarynnyy.

Quick questions related to the current implementation:

  • Can a command have multiple targets?
  • What is the "type": "che" what are the other possible types?
  • How can you stop a running task?

Few feedbacks:

  • I think the task panel does not provide a lot of help to the user to either see the task name or find quick actions which could be done on a task. Should we think about adding a dedicated header in the task panel to display those information?
  • For the preview, the interaction seems a bit strange to me. I don't understand why I have to go to different areas to select the preview I want to open. I think we should be displaying the list of previews from a dropdown menu from the status bar directly. Also if there is only a single preview, we should directly open it.
  • I'd love to open the preview directly in a new tab from Theia. While this is not required now, is that something we could be considering?

@azatsarynnyy
Copy link
Member

@slemeur thanks for your feedbacks.
Answers to your questions:

Can a command have multiple targets?

As I mentioned above, it can improved to make a target optional. In that case user could be asked to choose a machine to execute a command. Am I understand you correctly?

What is the "type": "che" what are the other possible types?

Task Type in Theia Task API allows different extensions to contribute it's own format of a task. I've described the examples of using it here.

How can you stop a running task?

At the moment - there's no way to stop a running task until machine-exec server provides killing an exec.

About feedbacks:
1/ Sorry but it's not clear for me what do you mean by "task panel"? Quick open widget or terminal widget?
2/ For Theia and VSCode it's a common approach to use the clickable elements on the status bar. Click on an element opens the quick open widget to pick an option.
3/ It's not obvious from the attached recording but actually it's opened in a new browser's tab.

@slemeur
Copy link
Contributor

slemeur commented May 31, 2018

Can a command have multiple targets?

As I mentioned above, it can improved to make a target optional. In that case user could be asked to choose a machine to execute a command. Am I understand you correctly?

I was more thinking about the ability to define more than one target for a particular task.

About feedbacks:
1/ Sorry but it's not clear for me what do you mean by "task panel"? Quick open widget or terminal widget?

The task panel is the panel where you are displaying the outputs of the task.

2/ For Theia and VSCode it's a common approach to use the clickable elements on the status bar. Click on an element opens the quick open widget to pick an option.

Ok, thanks. Do you feel it is correct? I personaly feel it disturbing in term of UX.

3/ It's not obvious from the attached recording but actually it's opened in a new browser's tab.

I was more meaning about displaying the preview inside of its own panel in the Theia UI directly - not in a new browser's tab. Just to know if it's doable?

@azatsarynnyy
Copy link
Member

I was more thinking about the ability to define more than one target for a particular task.

As I understood "multiple targets" means that a user can define the several targets and he/she should be asked to pick a one later. If it's correct then yes, it can be implemented as further improvements.

The task panel is the panel where you are displaying the outputs of the task.

I agree with you. Theia's Terminal widget is very simple and having Che's own output widget with additional functionality, which is Che specific, may be better solution.

Ok, thanks. Do you feel it is correct? I personaly feel it disturbing in term of UX.

As for me, I like the approach to use Commands Palette only. And if some command is used very often, a hotkey may be assigned. But I have no idea if it's already supported in Theia.

I was more meaning about displaying the preview inside of its own panel in the Theia UI directly - not in a new browser's tab. Just to know if it's doable?

Yes, it's doable.

@slemeur
Copy link
Contributor

slemeur commented Jun 1, 2018

Thanks for your answer @azatsarynnyy !

@azatsarynnyy
Copy link
Member

upd: The PR for the Theia Task API has been opened in Theia upstream - eclipse-theia/theia#2086. I continue working on finalizing the Che Theia Task extension.

@azatsarynnyy
Copy link
Member

There're the received feedbacks which we can consider to do as the follow-up improvements:

  • a command may have the multiple targets defined;
  • a command's target can be optional;
  • add a dedicated header in the terminal widget for displaying some info about Che task: target, preview URL etc.;
  • displaying the preview inside of its own panel in the Theia UI directly - not in a new browser's tab.

@azatsarynnyy
Copy link
Member

@vparfonov @slemeur
I've created the issues for the first three items and linked it to the epic #8382. I'm not sure about the 4th.

@slemeur
Copy link
Contributor

slemeur commented Jun 25, 2018

Thanks @azatsarynnyy !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

No branches or pull requests

3 participants