Skip to content

Commit

Permalink
Merge pull request PowerShell#2023 from PowerShell/andschwa/fix-outpu…
Browse files Browse the repository at this point in the history
…t-bug

Fix disappearing output in PowerShell 5.1
  • Loading branch information
andyleejordan committed May 12, 2023
2 parents 3401a2d + 9d2a151 commit 14f2be8
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 31 deletions.
4 changes: 3 additions & 1 deletion src/PowerShellEditorServices.Hosting/EditorServicesLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,9 @@ private void LogHostInformation()
private static string GetPSOutputEncoding()
{
using SMA.PowerShell pwsh = SMA.PowerShell.Create();
return pwsh.AddScript("$OutputEncoding.EncodingName", useLocalScope: true).Invoke<string>()[0];
return pwsh.AddScript(
"[System.Diagnostics.DebuggerHidden()]param() $OutputEncoding.EncodingName",
useLocalScope: true).Invoke<string>()[0];
}

// TODO: Deduplicate this with VersionUtils.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str

// Evaluate the expression to get back a PowerShell object from the expression string.
// This may throw, in which case the exception is propagated to the caller
PSCommand evaluateExpressionCommand = new PSCommand().AddScript(value);
PSCommand evaluateExpressionCommand = new PSCommand().AddScript($"[System.Diagnostics.DebuggerHidden()]param() {value}");
IReadOnlyList<object> expressionResults = await _executionService.ExecutePSCommandAsync<object>(evaluateExpressionCommand, CancellationToken.None).ConfigureAwait(false);
if (expressionResults.Count == 0)
{
Expand Down Expand Up @@ -500,7 +500,7 @@ public async Task<VariableDetails> EvaluateExpressionAsync(
bool writeResultAsOutput,
CancellationToken cancellationToken)
{
PSCommand command = new PSCommand().AddScript(expressionString);
PSCommand command = new PSCommand().AddScript($"[System.Diagnostics.DebuggerHidden()]param() {expressionString}");
IReadOnlyList<PSObject> results;
try
{
Expand Down Expand Up @@ -799,7 +799,7 @@ private async Task FetchStackFramesAsync(string scriptNameOverride)

// PSObject is used here instead of the specific type because we get deserialized
// objects from remote sessions and want a common interface.
PSCommand psCommand = new PSCommand().AddScript($"[Collections.ArrayList]{callStackVarName} = @(); {getPSCallStack}; {returnSerializedIfInRemoteRunspace}");
PSCommand psCommand = new PSCommand().AddScript($"[System.Diagnostics.DebuggerHidden()]param() [Collections.ArrayList]{callStackVarName} = @(); {getPSCallStack}; {returnSerializedIfInRemoteRunspace}");
IReadOnlyList<PSObject> results = await _executionService.ExecutePSCommandAsync<PSObject>(psCommand, CancellationToken.None).ConfigureAwait(false);

IEnumerable callStack = isRemoteRunspace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public async Task<EvaluateResponseBody> Handle(EvaluateRequestArguments request,
if (isFromRepl)
{
await _executionService.ExecutePSCommandAsync(
new PSCommand().AddScript(request.Expression),
new PSCommand().AddScript($"[System.Diagnostics.DebuggerHidden()]param() {request.Expression}"),
cancellationToken,
new PowerShellExecutionOptions { WriteOutputToHost = true, ThrowOnError = false, AddToHistory = true }).HandleErrorsAsync(_logger).ConfigureAwait(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public static PowerShellVersionDetails GetVersionDetails(ILogger logger, PowerSh
try
{
Hashtable psVersionTable = pwsh
.AddScript("$PSVersionTable", useLocalScope: true)
.AddScript("[System.Diagnostics.DebuggerHidden()]param() $PSVersionTable", useLocalScope: true)
.InvokeAndClear<Hashtable>()
.FirstOrDefault();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public async Task<ExpandAliasResult> Handle(ExpandAliasParams request, Cancellat
{
const string script = @"
function __Expand-Alias {
[System.Diagnostics.DebuggerHidden()]
param($targetScript)
[ref]$errors=$null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ internal class ShowHelpHandler : IShowHelpHandler

public async Task<Unit> Handle(ShowHelpParams request, CancellationToken cancellationToken)
{
// TODO: Refactor to not rerun the function definition every time.
const string CheckHelpScript = @"
[System.Diagnostics.DebuggerHidden()]
[CmdletBinding()]
param (
[String]$CommandName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,39 @@
using System.Collections.ObjectModel;
using System.Management.Automation;
using System.Management.Automation.Host;
using System.Reflection;
using System.Security;
using Microsoft.Extensions.Logging;
using Microsoft.PowerShell.EditorServices.Utility;

namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Host
{
internal class EditorServicesConsolePSHostUserInterface : PSHostUserInterface, IHostUISupportsMultipleChoiceSelection
{
private readonly PSHostUserInterface _underlyingHostUI;

private static readonly Action<PSHostUserInterface, bool> s_setTranscribeOnlyDelegate;

/// <summary>
/// We use a ConcurrentDictionary because ConcurrentHashSet does not exist, hence the value
/// is never actually used, and `WriteProgress` must be thread-safe.
/// </summary>
private readonly ConcurrentDictionary<(long, int), object> _currentProgressRecords = new();

static EditorServicesConsolePSHostUserInterface()
{
if (VersionUtils.IsPS5)
{
PropertyInfo transcribeOnlyProperty = typeof(PSHostUserInterface)
.GetProperty("TranscribeOnly", BindingFlags.NonPublic | BindingFlags.Instance);

MethodInfo transcribeOnlySetMethod = transcribeOnlyProperty.GetSetMethod(nonPublic: true);

s_setTranscribeOnlyDelegate = (Action<PSHostUserInterface, bool>)Delegate.CreateDelegate(
typeof(Action<PSHostUserInterface, bool>), transcribeOnlySetMethod);
}
}

public EditorServicesConsolePSHostUserInterface(
ILoggerFactory loggerFactory,
PSHostUserInterface underlyingHostUI)
Expand Down Expand Up @@ -70,7 +88,7 @@ public override void WriteProgress(long sourceId, ProgressRecord record)
_underlyingHostUI.WriteProgress(sourceId, record);
}

public void ResetProgress()
internal void ResetProgress()
{
// Mark all processed progress records as completed.
foreach ((long sourceId, int activityId) in _currentProgressRecords.Keys)
Expand All @@ -87,6 +105,17 @@ public void ResetProgress()
// TODO: Maybe send the OSC sequence to turn off progress indicator.
}

// This works around a bug in PowerShell 5.1 (that was later fixed) where a running
// transcription could cause output to disappear since the `TranscribeOnly` property was
// accidentally not reset to false.
internal void DisableTranscribeOnly()
{
if (VersionUtils.IsPS5)
{
s_setTranscribeOnlyDelegate(_underlyingHostUI, false);
}
}

public override void WriteVerboseLine(string message) => _underlyingHostUI.WriteVerboseLine(message);

public override void WriteWarningLine(string message) => _underlyingHostUI.WriteWarningLine(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ internal class PsesInternalHost : PSHost, IHostSupportsInteractiveSession, IRuns
{
internal const string DefaultPrompt = "> ";

private static readonly PSCommand s_promptCommand = new PSCommand().AddCommand("prompt");

private static readonly PropertyInfo s_scriptDebuggerTriggerObjectProperty;

private readonly ILoggerFactory _loggerFactory;
Expand Down Expand Up @@ -474,7 +476,19 @@ public void InvokeDelegate(string representation, ExecutionOptions executionOpti
public IReadOnlyList<TResult> InvokePSCommand<TResult>(PSCommand psCommand, PowerShellExecutionOptions executionOptions, CancellationToken cancellationToken)
{
SynchronousPowerShellTask<TResult> task = new(_logger, this, psCommand, executionOptions, cancellationToken);
return task.ExecuteAndGetResult(cancellationToken);
try
{
return task.ExecuteAndGetResult(cancellationToken);
}
finally
{
// At the end of each PowerShell command we need to reset PowerShell 5.1's
// `TranscribeOnly` property to avoid a bug where output disappears.
if (UI is EditorServicesConsolePSHostUserInterface ui)
{
ui.DisableTranscribeOnly();
}
}
}

public void InvokePSCommand(PSCommand psCommand, PowerShellExecutionOptions executionOptions, CancellationToken cancellationToken) => InvokePSCommand<PSObject>(psCommand, executionOptions, cancellationToken);
Expand Down Expand Up @@ -1026,10 +1040,8 @@ internal string GetPrompt(CancellationToken cancellationToken)
string prompt = DefaultPrompt;
try
{
// TODO: Should we cache PSCommands like this as static members?
PSCommand command = new PSCommand().AddCommand("prompt");
IReadOnlyList<string> results = InvokePSCommand<string>(
command,
s_promptCommand,
executionOptions: new PowerShellExecutionOptions { ThrowOnError = false },
cancellationToken);

Expand Down Expand Up @@ -1207,7 +1219,18 @@ private Runspace CreateInitialRunspace(InitialSessionState initialSessionState)
return runspace;
}

// NOTE: This token is received from PSReadLine, and it _is_ the ReadKey cancellation token!
/// <summary>
/// This delegate is handed to PSReadLine and overrides similar logic within its `ReadKey`
/// method. Essentially we're replacing PowerShell's `OnIdle` handler since the PowerShell
/// engine isn't idle when we're sitting in PSReadLine's `ReadKey` loop. In our case we also
/// use this idle time to process queued tasks by executing those that can run in the
/// background, and canceling the foreground task if a queued tasks requires the foreground.
/// Finally, if and only if we have to, we run an artificial pipeline to force PowerShell's
/// own event processing.
/// </summary>
/// <param name="idleCancellationToken">
/// This token is received from PSReadLine, and it is the ReadKey cancellation token!
/// </param>
internal void OnPowerShellIdle(CancellationToken idleCancellationToken)
{
IReadOnlyList<PSEventSubscriber> eventSubscribers = _mainRunspaceEngineIntrinsics.Events.Subscribers;
Expand Down Expand Up @@ -1250,17 +1273,27 @@ internal void OnPowerShellIdle(CancellationToken idleCancellationToken)

// If we're executing a PowerShell task, we don't need to run an extra pipeline
// later for events.
runPipelineForEventProcessing = task is not ISynchronousPowerShellTask;
if (task is ISynchronousPowerShellTask)
{
// We don't ever want to set this to true here, just skip if it had
// previously been set true.
runPipelineForEventProcessing = false;
}
ExecuteTaskSynchronously(task, cancellationScope.CancellationToken);
}
}

// We didn't end up executing anything in the background,
// so we need to run a small artificial pipeline instead
// to force event processing
// to force event processing.
if (runPipelineForEventProcessing)
{
InvokePSCommand(new PSCommand().AddScript("0", useLocalScope: true), executionOptions: null, CancellationToken.None);
InvokePSCommand(
new PSCommand().AddScript(
"[System.Diagnostics.DebuggerHidden()]param() 0",
useLocalScope: true),
executionOptions: null,
CancellationToken.None);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public static SessionDetails GetFromPowerShell(PowerShell pwsh)
{
Hashtable detailsObject = pwsh
.AddScript(
$"@{{ '{Property_ComputerName}' = if ([Environment]::MachineName) {{[Environment]::MachineName}} else {{'localhost'}}; '{Property_ProcessId}' = $PID; '{Property_InstanceId}' = $host.InstanceId }}",
$"[System.Diagnostics.DebuggerHidden()]param() @{{ '{Property_ComputerName}' = if ([Environment]::MachineName) {{[Environment]::MachineName}} else {{'localhost'}}; '{Property_ProcessId}' = $PID; '{Property_InstanceId}' = $host.InstanceId }}",
useLocalScope: true)
.InvokeAndClear<Hashtable>()
.FirstOrDefault();
Expand Down
32 changes: 18 additions & 14 deletions src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,6 @@ public IEnumerable<SymbolReference> FindSymbolsInFile(ScriptFile scriptFile)
// asserting we should use a giant nested ternary.
private static string[] GetIdentifiers(string symbolName, SymbolType symbolType, CommandHelpers.AliasMap aliases)
{
if (symbolType is not SymbolType.Function)
{
return new[] { symbolName };
}

if (!aliases.CmdletToAliases.TryGetValue(symbolName, out List<string> foundAliasList))
{
return new[] { symbolName };
Expand All @@ -165,22 +160,31 @@ public async Task<IEnumerable<SymbolReference>> ScanForReferencesOfSymbolAsync(
return Enumerable.Empty<SymbolReference>();
}

// TODO: Should we handle aliases at a lower level?
CommandHelpers.AliasMap aliases = await CommandHelpers.GetAliasesAsync(
_executionService,
cancellationToken).ConfigureAwait(false);
// We want to handle aliases for functions, but we only want to do the work of getting
// the aliases when we must. We can't cache the alias list on first run else we won't
// support newly defined aliases.
string[] allIdentifiers;
if (symbol.Type is SymbolType.Function)
{
CommandHelpers.AliasMap aliases = await CommandHelpers.GetAliasesAsync(
_executionService,
cancellationToken).ConfigureAwait(false);

string targetName = symbol.Id;
if (symbol.Type is SymbolType.Function
&& aliases.AliasToCmdlets.TryGetValue(symbol.Id, out string aliasDefinition))
string targetName = symbol.Id;
if (aliases.AliasToCmdlets.TryGetValue(symbol.Id, out string aliasDefinition))
{
targetName = aliasDefinition;
}
allIdentifiers = GetIdentifiers(targetName, symbol.Type, aliases);
}
else
{
targetName = aliasDefinition;
allIdentifiers = new[] { symbol.Id };
}

await ScanWorkspacePSFiles(cancellationToken).ConfigureAwait(false);

List<SymbolReference> symbols = new();
string[] allIdentifiers = GetIdentifiers(targetName, symbol.Type, aliases);

foreach (ScriptFile file in _workspaceService.GetOpenedFiles())
{
Expand Down

0 comments on commit 14f2be8

Please sign in to comment.