Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Use terminal widget for output rendering in the task plugin #223

Merged
merged 9 commits into from
May 17, 2019

Conversation

AndrienkoAleksandr
Copy link
Contributor

@AndrienkoAleksandr AndrienkoAleksandr commented May 16, 2019

What does this PR do?

Proposed changes:

  • Use terminal widget for output rendering in the task plugin.
  • Fix terminal onDidClosed method for terminal plugin api.

What issues does this PR fix or reference?

#192

cwd: target.workingDir,
name: taskConfig.label,
shellPath: 'sh',
shellArgs: ['-c', "'" + taskConfig.command + "'"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we use string literals ?

Signed-off-by: Oleksandr Andriienko <[email protected]>
Signed-off-by: Oleksandr Andriienko <[email protected]>
Signed-off-by: Oleksandr Andriienko <[email protected]>
Signed-off-by: Oleksandr Andriienko <[email protected]>
@RomanNikitenko
Copy link
Member

@AndrienkoAleksandr
build_task

Is it possible to reuse widget for the same task if task was completed?
If so and it makes sense, we could improve this behavior, maybe in another issue

Copy link
Member

@RomanNikitenko RomanNikitenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndrienkoAleksandr
I tested your changes and tried to run a task in ws/dev container, but it always was run in theia-ide container.
Could you test this case?
Please see video https://youtu.be/qooQnIPSKNI

Signed-off-by: Oleksandr Andriienko <[email protected]>
Signed-off-by: Oleksandr Andriienko <[email protected]>
@AndrienkoAleksandr
Copy link
Contributor Author

@AndrienkoAleksandr
I tested your changes and tried to run a task in ws/dev container, but it always was run in theia-ide container.
Could you test this case?
Please see video https://youtu.be/qooQnIPSKNI

Thanks a lot. Fixed!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants