You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The current prompt APIs suffer from a few problems.
* The syncrhonous `ReadKey` method does not support cancellation.
* The syncrhonous `ReadKey` method returns a nullable `ConsoleKeyInfo` struct but `System.Console.ReadKey` can never return a nullable `ConsoleKeyInfo`.
* The asyncrhonous `ReadKeyAsync` method can return `null` only if cancellation has been requested. But this can never actually happen since [Fix deadlock when cancelling prompts (spectreconsole#1439)](spectreconsole#1439) was merged.
Here's are the problematic implementation (before this commit fixes it):
```csharp
public ConsoleKeyInfo? ReadKey(bool intercept)
{
return System.Console.ReadKey(intercept);
}
public async Task<ConsoleKeyInfo?> ReadKeyAsync(bool intercept, CancellationToken cancellationToken)
{
while (true)
{
if (cancellationToken.IsCancellationRequested)
{
return null;
}
if (System.Console.KeyAvailable)
{
break;
}
await Task.Delay(5, cancellationToken).ConfigureAwait(false);
}
return ReadKey(intercept);
}
```
Note that adding a `CancellationToken` parameter and returning `ConsoleKeyInfo` instead of `ConsoleKeyInfo?` is a breaking change since it modifies the signatures of the public `IAnsiConsoleInput` interface. But this should not be an issue since it's impossible to use another implentation than `DefaultInput` when used through `AnsiConsole.Create(AnsiConsoleSettings settings)`.
I have also searched for [implementers of IAnsiConsoleInput](https://grep.app/search?q=IAnsiConsoleInput) and I think this change won't break anything since nobody actually implemented `IAnsiConsoleInput`. Only exising implementations which have been udated are being used (at least across a half million public git repos).
The addition of the `CancellationToken` to `IPrompt.Show(IAnsiConsole console, CancellationToken cancellationToken = default)` is also a breaking change but it should be mitigated since it has bee introduced with a default value.
0 commit comments