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

process: display process tasks in the terminal #5895

Merged
merged 1 commit into from
Aug 13, 2019

Conversation

paul-marechal
Copy link
Member

@paul-marechal paul-marechal commented Aug 8, 2019

What it does

Displays process task output in a terminal like VS Code.

Fixes #5908.

How to test

You can use the following task configuration to see that process task output is now displayed in a terminal:

{
    "tasks": [
        {
            "label": "[Task] Echo a string",
            "type": "process",
            "cwd": "${workspaceFolder}",
            "command": "bash",
            "args": [
                "-c",
                "sleep 3 && echo 1 2 3"
            ]
        }
    ]
}

Review checklist

Reminder for reviewers

@vince-fugnitto
Copy link
Member

The PR breaks the command Task: Show Running Tasks...

Steps to Reproduce:

  1. open the folder theia/packages/task/test-resources as a workspace
  2. run multiple tasks shell: long running test task (ex: 2)
  3. execute the command Task: Show Running Task... and select a task from the dropdown
  4. the terminal displaying the output of the task is not activated

@akosyakov
Copy link
Member

akosyakov commented Aug 9, 2019

Is there a related issue for it? please reference it with fix in PR body and commit

@akosyakov akosyakov added process issues related to the process extension tasks issues related to the task system labels Aug 9, 2019
`process` tasks aren't displayed in the terminal, whereas it is
displayed as such in VS Code.

This commit makes it so `process` tasks are displayed the same way
`shell` tasks are.

Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
@paul-marechal paul-marechal force-pushed the mp/process-task-terminal branch from 7a7cb99 to 35ee65d Compare August 9, 2019 21:35
@paul-marechal
Copy link
Member Author

@vince-fugnitto should be fixed.

@paul-marechal
Copy link
Member Author

To people reviewing and that are familiar with VS Code tasks system: Should we rather display any task output in the terminals? Are there cases where the output is not displayed?

@elaihau
Copy link
Contributor

elaihau commented Aug 11, 2019

To people reviewing and that are familiar with VS Code tasks system: Should we rather display any task output in the terminals? Are there cases where the output is not displayed?

there is a showOutput flag we can use to toggle https://stackoverflow.com/a/38723286
also, isBackground flag is kind-of related (search "isBackground" from https://code.visualstudio.com/docs/editor/tasks)

Copy link
Contributor

@elaihau elaihau left a comment

Choose a reason for hiding this comment

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

I tested the change in both single and multi root workspace. I verified the followings:

  • task can be started and terminated
  • attach task worked
  • recent tasks are displayed in the quick open properly
  • detected tasks are not affected.

Thank you for the work !

@akosyakov
Copy link
Member

there is a showOutput flag we can use to toggle https://stackoverflow.com/a/38723286
also, isBackground flag is kind-of related (search "isBackground" from https://code.visualstudio.com/docs/editor/tasks)

@elaihau please make sure to file Theia issues to support these features

@vince-fugnitto
Copy link
Member

@vince-fugnitto should be fixed.

I confirmed that it still works, thanks.

Copy link
Contributor

@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.

I tested the changes and tried to run tasks:

  • with process type
  • with shell type
  • npm tasks

I got output for those tasks, so it works well for me.

@paul-marechal paul-marechal merged commit e3dfbea into master Aug 13, 2019
@paul-marechal paul-marechal deleted the mp/process-task-terminal branch August 13, 2019 14:35
@elaihau
Copy link
Contributor

elaihau commented Aug 17, 2019

Sure Anton. I created #5974

@elaihau
Copy link
Contributor

elaihau commented Aug 17, 2019

I am not sure if Theia needs isBackground though. Is task-in-theia not always running in the background unless we terminate it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process issues related to the process extension tasks issues related to the task system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

task: process output is not displayed in a terminal
5 participants