Also need to check that PS error stream is empty.

will add

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.

Need newline in the end of file.

will add

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

will change

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.

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.

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.

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?

Will fix

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

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

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

will fix

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.

Makes sense, will fix.

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.

Will fix

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

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

yup

These changes now can be reverted.

Will revert

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.

I see. I'll try that.