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

Help users to migrate to new tasks.json version 2.0.0. #22065

Closed
dbaeumer opened this issue Mar 6, 2017 · 20 comments
Closed

Help users to migrate to new tasks.json version 2.0.0. #22065

dbaeumer opened this issue Mar 6, 2017 · 20 comments
Assignees
Labels
plan-item VS Code - planned item for upcoming tasks Task system issues

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Mar 6, 2017

  • VSCode Version: 1.1.0

To make use of the task execution in the terminal users need to migrate to version 2.0.0 of tasks.json. Support this migration path with a automatic conversion of version 0.1.0 to 2.0.0.

@dbaeumer dbaeumer self-assigned this Mar 6, 2017
@dbaeumer dbaeumer added the plan-item VS Code - planned item for upcoming label Mar 6, 2017
@dbaeumer dbaeumer added this to the March 2017 milestone Mar 6, 2017
@kieferrm kieferrm mentioned this issue Mar 6, 2017
58 tasks
@dbaeumer
Copy link
Member Author

In this simple example change version to 2.0.0

@dbaeumer dbaeumer modified the milestones: April 2017, March 2017 Mar 23, 2017
@dbaeumer
Copy link
Member Author

Moving to April. We added API for task in this milestone and we need to wait at least one release before we can finalized the API. Until then we shouldn't finalized the 2.0 version of tasks.

@dbaeumer dbaeumer modified the milestones: Mai 2017, April 2017 Apr 13, 2017
@dbaeumer dbaeumer added the tasks Task system issues label Apr 13, 2017
@Spown
Copy link

Spown commented Apr 13, 2017

also docs have no mention of 2.0.0 either.

@dbaeumer
Copy link
Member Author

Agree, reason being that this is not the default yet. It got mentioned in the release notes.

@rkeithhill
Copy link

rkeithhill commented May 31, 2017

Any idea why a problem matcher under version 0.1.0 correctly finds a single error in task output but when I switch the tasks.json to 2.0.0 the same problem matcher reports the single error multiple times?

image

cc @daviwil - I was trying to integrate the Pester problem matcher we ship with the Examples into the PowerShell extension's package.json as a contributed problem matcher named "pesterError".

@dbaeumer
Copy link
Member Author

@rkeithhill nothing has changed with problem matchers and the terminal engine is around since 2 releases. Do you have steps to reproduce (best a workspace to clone from). Then I will look into this.

@rkeithhill
Copy link

rkeithhill commented May 31, 2017

Install the PowerShell extension (1.1.0), execute the command PowerShell: Open Examples Folder, edit the file Tests\PathProcessing.Tests.ps1 - change line 77 from:

        $files[1] | Should Be "$WorkspaceRoot\SampleModule.psd1"

to

        $files[2] | Should Be "$WorkspaceRoot\SampleModule.psd1"

That will introduce a Pester test error. Then run the task named test. That should be enough so you see the multiple errors for the one Pester error. If you change the tasks.json version back to 0.1.0, reload the window and run the test task again, you should see just the single error reported in the Problems window.

P.S. Thanks for looking at this. :-)

@dbaeumer dbaeumer modified the milestones: June 2017, May 2017 Jun 1, 2017
@dbaeumer dbaeumer modified the milestones: On Deck, June 2017 Jun 29, 2017
@rkeithhill
Copy link

Just tried this again in today's Insiders build and the problem is still there. In fact, it seems worse (even more duplicate problem listings).

@dbaeumer
Copy link
Member Author

dbaeumer commented Jul 6, 2017

I looked into it today and I can reproduce this in two ways: no problem is recognized and a lot of problems are recognized. From a first investigation this comes form the 'progress overlay' the runner is showing and the fact that lines reported multiple times by the underlying terminal. Need to come up with a test case to forward this to the terminal implementer.

@dbaeumer
Copy link
Member Author

dbaeumer commented Jul 6, 2017

I debugged this further and the problem is that Pester when run in a 'terminal' (a tty/pty is attached) outputs lines n times. This depends on the progress overlay and the height of the terminal.

When run in the output panel we simply spawn a child process without a tty and then no progress or ANSI control characters are emitted.

I looked in Pester and if I can disable this 'stylish' output but wasn't successful. @daviwil @rkeithhill do you know such an option.

I can also look into de-duping this in VS Code until we have #29840

@rkeithhill
Copy link

Cool! Yes, we do have a way to eliminate the progress output. I can change the Pester task args from this:

            "args": [
                "Write-Host 'Invoking Pester...'; Invoke-Pester -PesterOption @{IncludeVSCodeMarker=$true};",
                "Invoke-Command { Write-Host 'Completed Test task in task runner.' }"
            ],

to this:

            "args": [
                "Write-Host 'Invoking Pester...'; $ProgressPreference = 'SilentlyContinue'; Invoke-Pester -PesterOption @{IncludeVSCodeMarker=$true};",
                "Invoke-Command { Write-Host 'Completed Test task in task runner.' }"
            ],

Long term, it would be nice if the tasks/terminal/problems combo could handle these progress overlays but I can appreciate the difficulties in doing so.

@daviwil A VSCode-based UI for progress would also eliminate this issue ... if only PowerShell progress reporting wasn't so complex.

rkeithhill added a commit to rkeithhill/vscode-powershell that referenced this issue Jul 6, 2017
When tasks is set to 2.0.0, the tasks run in the terminal and in this case, Pester outputs a bunch of progress text to the terminal.  This results in duplicate failures being found by the problem matcher.

Work around this problem for now by eliminating the Pester progress output.  For more details see - microsoft/vscode#22065 (comment)
@rkeithhill
Copy link

rkeithhill commented Jul 6, 2017

@dbaeumer BTW now that this problem has a workaround, I see another issue. If the terminal isn't wide enough, the output that the $pester problem matcher looks at changes. That is, what should be a single line with filename and line number on it, gets wrapped to two lines. The problem matcher no longer matches and the problem (test failure) doesn't get flagged. Perhaps I can adjust the problem matcher to handle this but I have to think other folks will run into this issue. cc @daviwil

Hmm, ah I see. The terminal just reports its current width to PowerShell and PowerShell injects a newline so the output spans two lines. For shells like PowerShell it might be nice if the terminal could provide a minimum width option, say 120 chars wide, and provide scroll bars when the terminal gets narrower than that. A lot of PowerShell output (for better or worse) assumes a console that is at least 120 chars wide.

daviwil pushed a commit to PowerShell/vscode-powershell that referenced this issue Jul 6, 2017
When tasks is set to 2.0.0, the tasks run in the terminal and in this case, Pester outputs a bunch of progress text to the terminal.  This results in duplicate failures being found by the problem matcher.

Work around this problem for now by eliminating the Pester progress output.  For more details see - microsoft/vscode#22065 (comment)
@Tyriar
Copy link
Member

Tyriar commented Jul 6, 2017

@rkeithhill I think we should be able to determine when a line is wrapped by PowerShell eventually and unwrap it. I just added this recently when copying wrapped lines https://twitter.com/Tyriar/status/875044402867163137

You could verify whether this method should work in PowerShell by trying to copy and paste a wrapped line in the latest Insiders.

@rkeithhill
Copy link

@Tyriar Just tried on today's Insider build and it doesn't seem to work for PowerShell. :-(

Open PowerShell in the terminal and try executing dir in a narrow terminal window and copy/paste the wrapped output.

The difference may be due to PowerShell's formatting & output engine. PowerShell examines the host's width and adjusts its output - inserting newlines (or truncating with ...) and even placing subsequent lines of data in the appropriate columns e.g.:

-a----         9/9/2015   8:45 PM           2435 UIAutomation_record
                                                 ing_short_9_9_2015_
                                                 8_35_PM.ps1
-a----         6/6/2017   9:33 PM             10 unicode.txt
-a----         7/4/2017   9:03 PM             31 Untitled-1.ps1
-a----         7/6/2017   9:54 AM            183 _lesshst

In the case of Bash, is it the "terminal" that inserts the newline to force wrapping or does the newline come from Bash? I suspect it is the former, which would make PowerShell behave fundamentally different than Bash in this regard.

@Tyriar
Copy link
Member

Tyriar commented Jul 6, 2017

@rkeithhill yeah maybe that won't work then, for bash it's the pty that does this which is at a higher level than bash. For Windows it works by running PowerShell through https://github.com/rprichard/winpty which allows us to plug in using Unix-like APIs, there are many issues that come out of that though as the Windows APIs around this are not particularly compatible.

This is particularly problematic as there are spaces padding it out:

-a----         9/9/2015   8:45 PM           2435 UIAutomation_record
                                                 ing_short_9_9_2015_
                                                 8_35_PM.ps1

You would either need to tell PS how to format (preferable) or the problem matcher would need to know about how PS is formatting these things (less preferable).

On the topic of scroll bars, it would be possible but I'm not sure we would want to go down that route as it's a lot of work/complexity for not much payoff imo. Plus it would further diverge how the terminal behaves on Windows which was always one of the core benefits of VS Code's terminal (how you configure/use it the same on all OS's).

@rkeithhill
Copy link

You would either need to tell PS how to format (preferable)

Well if this particular command wrote to the PowerShell output stream I could do that but no, it writes directly to the host so it can "color" the text.

@dbaeumer
Copy link
Member Author

dbaeumer commented Jul 7, 2017

@Tyriar just to clarify for me: the terminal itself is no longer inserting new lines into the emitted data to wrap lines. This is a pure visual thing. Things are different in PowerShell since there the engine itself formats to the width of the screen (not simple line wrapping)

@Tyriar
Copy link
Member

Tyriar commented Jul 7, 2017

@dbaeumer yeah I think that's right and not sure what we can do about it, apart from somehow getting PS not to output like that (some script arg?) or getting tasks to understand this layout better.

@dbaeumer
Copy link
Member Author

The 2.0.0 task system is the default for quite some time. I will close the issue since we don't plan to provide an automatic conversion right now.

If you have problem with the 2.0.0 runner please open a new issue so that we can track it separately. Having a global item will not help addressing them.

@dbaeumer dbaeumer removed this from the On Deck milestone Sep 24, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
plan-item VS Code - planned item for upcoming tasks Task system issues
Projects
None yet
Development

No branches or pull requests

5 participants