GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Enable transcription of native commands on non-Windows by SteveL-MSFT · Pull Request #4871 · PowerShell/PowerShell

Conversation

@SteveL-MSFT

@SteveL-MSFT Steve Lee (SteveL-MSFT) commented Sep 20, 2017

Copy link
Copy Markdown
Member

Transcription was relying on reading the screen buffer to record output from native commands.
This resulted in an unhandled exception calling an unimplemented api on non-Windows.
Fix is to default to using redirected output if reading the screen buffer is not supported.

Fix #1920

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT Steve Lee (SteveL-MSFT) assigned Dongbo Wang (daxian-dbw) Sep 20, 2017
@SteveL-MSFT
Steve Lee (SteveL-MSFT) requested a review from Andrew (anmenaga) September 20, 2017 04:26
@SteveL-MSFT
Steve Lee (SteveL-MSFT) requested review from Bruce Payette (BrucePay), Dongbo Wang (daxian-dbw), Jason Shirk (lzybkr) and sergei (vors) as code owners September 20, 2017 04:26
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Sep 20, 2017
anmenaga
Andrew (anmenaga) suggested changes Sep 20, 2017
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 Outdated

@anmenaga Andrew (anmenaga) Sep 20, 2017

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Also need to check that PS error stream is empty.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 20, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

will add

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 20, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Actually, there's no reason to follow that pattern, I'm rewriting it as a scriptblock and any errors should just show up in the test run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 Outdated

@anmenaga Andrew (anmenaga) Sep 20, 2017

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Need newline in the end of file.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 20, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

will add

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Sep 20, 2017
View reviewed changes
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 20, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This can be made a static check so that we do it only once. And _scrapeHostOutput can be renamed to s_supportScreenScrape (or something similar).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 22, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

will change

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 20, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why do we want redirectInput to be true by default? Even if we are transcribing and screen scrape is not supported, we shouldn't need to always redirect the input.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 20, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

And form line 442 below, it indicates that we transcribe for native command only if _runStandAlone is true. Now it looks like you are assuming we can do transcribe regardless of _runStandAlone. That seems not right.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 22, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yes, redirectInput shouldn't be true by default. Will fix.

As for _runStandAlone, my understanding of the code is that if not running standalone, output is redirected and will be captured in the transcript. However, if running standalone, the output is screen scraped and put into the transcript. So it seems that regardless if running standalone, if screen scraping is not supported, the only way to transcribe the output is to redirect it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 20, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggestion: this.Command.Context.EngineHostInterface.UI.IsTranscribing is used at 3 places in this class. Maybe we can have an instance field to cache the value? Like _isTranscribing = this.Command.Context.EngineHostInterface.UI.IsTranscribing in the constructor?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 22, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Will fix

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 20, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

from line 444 below, it looks like a HostException also indicates The host doesn't support scraping via its RawUI interface ...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 22, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think the catch for HostException was a mistake. Going through the code in the debugger, that exception isn't thrown.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 20, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

By putting the call to GetBufferContents in the constructor, it's possible that an arbitrary exception will be thrown from GetBufferContents in the constructor. When that happens, it would be wrapped to an ApplicationFailedException properly prior to this change, but after this change, the NativeCommandProcessor would just fail to be constructed, and thus I guess the error would look obscure too. So, I'm not sure if it's appropriate to put this in the constructor ...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 22, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

will fix

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Sep 20, 2017

Copy link
Copy Markdown
Member

Fix is to default to using redirected output if reading the screen buffer is not supported.

Steve Lee (@SteveL-MSFT) Could you please explain a little why redirecting the output/error would support transcribing in case that screen scraping is not supported? (just curious how it's done in that case)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Sep 21, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) from what I can tell in the code, anything that gets redirected gets transcribed anyways
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT
Steve Lee (SteveL-MSFT) requested review from Hemant Mahawar (HemantMahawar), James Truher (JamesWTruher), Paul Higinbotham (PaulHigin), Travis Plunk (TravisEz13), Aditya Patwardhan (adityapatwardhan), Dan Travison (dantraMSFT) and Joey Aiello (joeyaiello) as code owners September 23, 2017 00:06
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the mac-transcript branch from 722b161 to c72288d Compare September 23, 2017 00:07
@vors
sergei (vors) removed their request for review September 25, 2017 00:39
daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Sep 25, 2017
View reviewed changes
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 25, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think setting redirections for transcribing purpose should be done in the method CalculateIORedirection, and IMO it should be done at the end of that method -- after the existing logic of that method has run. For example, in case of running on the server side, redirectInput should always set to be true. Code like that should run even though screen scraping is not supported.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 25, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Makes sense, will fix.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 25, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think there is a change of behavior here. Assuming _isTranscribing == $true and screen scraping is supported,

  • before the change, _scrapeHostOutput is true only if _runStandAlone is true, so this block will run only the native command is running standalone.
  • after the change, even if it's not running standalone (i.e. standard input/error redirected), this block of code still will run when transcribing, and in such case, _startPosition is not even initialized.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 25, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Will fix

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Sep 25, 2017
View reviewed changes
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 25, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe remove the else? The else if (Platform.IsWindowsDesktop && IsConsoleApplication(this.Path)) block doesn't necessarily set redirectOutput and redirectError to true.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 25, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

And please add a comment that explains why we need to set output/error redirection in such case.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 25, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

yup

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 25, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

These changes now can be reverted.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 25, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Will revert

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Sep 25, 2017
View reviewed changes
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Sep 25, 2017

Copy link
Copy Markdown
Member

Andrew (@anmenaga) Can you please take a look again?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the mac-transcript branch from 015ca77 to f769df1 Compare September 25, 2017 23:58
@msftgits Microsoft GitHub User (msftgits) removed the cla-not-required label Sep 26, 2017
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the mac-transcript branch from f769df1 to b63b374 Compare September 26, 2017 18:54
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Sep 26, 2017

Copy link
Copy Markdown
Member Author

Mark Kraus (@markekraus) the test failures are indicating the WebListener didn't start within the timeout. Is this something you're familiar with?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markekraus

Mark Kraus (markekraus) commented Sep 26, 2017

Copy link
Copy Markdown
Contributor

Steve Lee (@SteveL-MSFT) News to me. I will take a look.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
anmenaga
Andrew (anmenaga) approved these changes Sep 26, 2017
View reviewed changes
@markekraus

Mark Kraus (markekraus) commented Sep 26, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Steve Lee (@SteveL-MSFT) This appears to be related to the change in this PR. It looks native execution inside a job is broken. WebListener runs dotnet WebListener.dll in a Start-Job block. From your PR I can manually run the WebListener outside of a job in PowerShell. But inside a job it fails. But it's not limited to WebListener:

$Job = Start-Job {cmd.exe /c echo "test" }
$Job | Wait-Job | fl * -force

$Job.ChildJobs[0].Output

expected behavior (working in beta.7):

State         : Completed
HasMoreData   : True
StatusMessage :
Location      : localhost
Command       : cmd.exe /c echo "test"
JobStateInfo  : Completed
Finished      : System.Threading.ManualResetEvent
InstanceId    : 7d654dd9-ba42-48f0-be72-9e91863e7286
Id            : 1
Name          : Job1
ChildJobs     : {Job2}
PSBeginTime   : 9/26/2017 3:41:02 PM
PSEndTime     : 9/26/2017 3:41:02 PM
PSJobTypeName : BackgroundJob
Output        : {}
Error         : {}
Progress      : {}
Verbose       : {}
Debug         : {}
Warning       : {}
Information   : {}

test

Actual behavior:

State         : Failed
HasMoreData   : False
StatusMessage :
Location      : localhost
Command       : cmd.exe /c echo "test"
JobStateInfo  : Failed
Finished      : System.Threading.ManualResetEvent
InstanceId    : 58c30632-c4a1-41e3-b276-5aa9b24db3b0
Id            : 3
Name          : Job3
ChildJobs     : {Job4}
PSBeginTime   : 9/26/2017 3:42:41 PM
PSEndTime     : 9/26/2017 3:42:42 PM
PSJobTypeName : BackgroundJob
Output        : {}
Error         : {}
Progress      : {}
Verbose       : {}
Debug         : {}
Warning       : {}
Information   : {}
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Sep 26, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Mark Kraus (@markekraus) thanks! I'll take a look.

The problem is that when run as a job obviously there's no console, so some of the console calls even on Windows throws other exceptions (and it's not just the Host.HostException from the old code) when the code is checking if screen scraping is supported. Added a test to test start-job with native command to cover this.

Dongbo Wang (@daxian-dbw) Andrew (@anmenaga) can you take another look?
markekraus reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Steve Lee (SteveL-MSFT) added 6 commits September 26, 2017 15:36
@SteveL-MSFT
handle exception for non-Windows where reading the screen buffer is n…
f39b1d6 
…ot supported

since screen buffer reading isn't supported, default to using redirected output
@SteveL-MSFT
address PR feedback
5ad677f
@SteveL-MSFT
address Dongbo's PR feedback
7373ea5
@SteveL-MSFT
[feature]
c68cb39 
moved setting redirection due to transcription to within CalculateIORedirection() function
ensure screen scrapping only happens when running standalone
@SteveL-MSFT
[feature]
0f2b52a 
address PR feedback from Dongbo
@SteveL-MSFT
[feature]
2ff64d8 
added test for start-job and native commands
catch all exceptions trying to read the buffer
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the mac-transcript branch from b63b374 to 2ff64d8 Compare September 26, 2017 22:50
daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Sep 27, 2017
View reviewed changes
Comment thread src/System.Management.Automation/engine/NativeCommandProcessor.cs Outdated
catch (Exception)
{
// screen scraping not supported on non-Windows or as job
}

@daxian-dbw Dongbo Wang (daxian-dbw) Sep 27, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think this check can still be done only once -- we do this check in CalculateIORedirection and only if _runStandAlone is true. Here the the code I propose:

private void CalculateIORedirection(out bool redirectOutput, out bool redirectError, out bool redirectInput)
{
    ...

    if (NativeCommandProcessor.IsServerSide)
    {
        redirectInput = true;
        redirectOutput = true;
        redirectError = true;
    }
    else if (Platform.IsWindowsDesktop && IsConsoleApplication(this.Path))
    {
        // On Windows desktops, if the command to run is a console application,
        // then allocate a console if there isn't one attached already...
        ConsoleVisibility.AllocateHiddenConsole();

        if (ConsoleVisibility.AlwaysCaptureApplicationIO)
        {
            redirectOutput = true;
            redirectError = true;
        }
    }
    _runStandAlone = !redirectInput && !redirectOutput && !redirectError;

    // !!!!! NOTE !!!!! -- Unchanged until here

    if (_runStandAlone)
    {
        if (s_supportScreenScrape == null)
        {
            // Call GetBufferContents() to check and set s_supportScreenScrape to true or false accordingly.
        }

        // if screen scraping isn't supported, we enable redirection so that the output is still transcribed
        // as redirected output is always transcribed
        if (_isTranscribing && (false == s_supportScreenScrape))
        {
            redirectOutput = true;
            redirectError = true;
            _runStandAlone = false;
        }
    }
}

So the check of screen scraping support is done only if we are running the native command as standalone application.

In case of running it in a Job, I think the standard input and error are already redirected, so nothing needs to be done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Sep 27, 2017

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I see. I'll try that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT
[feature]
bb5767d 
refactor code
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Sep 28, 2017
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 99e3fe5 into PowerShell:master Sep 29, 2017
@SteveL-MSFT
Steve Lee (SteveL-MSFT) deleted the mac-transcript branch October 26, 2018 21:33
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@SteveL-MSFT @daxian-dbw
Enable transcription of native commands on non-Windows (PowerShell#4871)
ae84094 
Transcription was relying on reading the screen buffer to record output from native commands.
This resulted in an unhandled exception calling an unimplemented API on non-Windows.
The fix is to use redirected output/error if reading the screen buffer is not supported.
We check whether screen scraping is supported or not only when the application is running standalone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@BrucePay Bruce Payette (BrucePay) Awaiting requested review from BrucePay
@lzybkr Jason Shirk (lzybkr) Awaiting requested review from lzybkr
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@dantraMSFT Dan Travison (dantraMSFT) Awaiting requested review from dantraMSFT
@HemantMahawar Hemant Mahawar (HemantMahawar) Awaiting requested review from HemantMahawar
@JamesWTruher James Truher (JamesWTruher) Awaiting requested review from JamesWTruher
@joeyaiello Joey Aiello (joeyaiello) Awaiting requested review from joeyaiello
@PaulHigin Paul Higinbotham (PaulHigin) Awaiting requested review from PaulHigin
@TravisEz13 Travis Plunk (TravisEz13) Awaiting requested review from TravisEz13
+1 more reviewer
@anmenaga Andrew (anmenaga) anmenaga approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@daxian-dbw Dongbo Wang (daxian-dbw)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@SteveL-MSFT @daxian-dbw @markekraus @anmenaga @msftclas @msftgits
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.