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 several problems.
First, the prompt APIs are fundamentally synchronous APIs. If we get down to the underlying implementation of all the prompt APIs, i.e. the `DefaultInput` class we can see some issues.
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);
}
```
* 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.
* The asyncrhonous `ReadKeyAsync` method is not actually an synchronous method, it's waiting for a key to be pressed in a loop, waiting 5 milliseconds before checking again if it can break out of the loop.
The proposed fix obsoletes the `ReadKeyAsync` method and add cancellation support to the synchronous `ReadKey` method through an optional `CancellationToken`. It also returns a non-nullable `ConsoleKeyInfo` making it clear that the only way to get out of this method is through cancellation. Then this change bubbles up to all the prompt APIs, also obsoleting the `IPrompt.ShowAsync` methods.
This is a better alternative to [Async overloads for AnsiConsole Prompt/Ask/Confirm (spectreconsole#1194)](spectreconsole#1194) where the actual need is having a `CancellationToken` to perform some cleanup and not having async prompt APIs.
Note that this is a breaking change since it modifies the signatures of the public `IAnsiConsoleInput` interface but that 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