Skip to content

Commit

Permalink
Obsolete the async prompt APIs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
0xced committed Sep 10, 2024
1 parent 78f3f80 commit 961af98
Show file tree
Hide file tree
Showing 15 changed files with 161 additions and 102 deletions.
3 changes: 3 additions & 0 deletions docs/input/prompts/multiselection.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ Highlights:
- Display multiple items for a user to scroll and choose from.
- Custom page sizes.
- Provide groups of selectable items.
Reference:
- T:Spectre.Console.MultiSelectionPrompt`1
- M:Spectre.Console.AnsiConsole.Prompt``1(Spectre.Console.IPrompt{``0},System.Threading.CancellationToken)
---

The `MultiSelectionPrompt` can be used when you want the user to select
Expand Down
2 changes: 1 addition & 1 deletion docs/input/prompts/selection.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Order: 1
Description: "The **SelectionPrompt** can be used when you want the user to select a single item from a provided list."
Reference:
- T:Spectre.Console.SelectionPrompt`1
- M:Spectre.Console.AnsiConsole.Prompt``1(Spectre.Console.IPrompt{``0})
- M:Spectre.Console.AnsiConsole.Prompt``1(Spectre.Console.IPrompt{``0},System.Threading.CancellationToken)
---

The `SelectionPrompt` can be used when you want the user to select
Expand Down
7 changes: 5 additions & 2 deletions src/Spectre.Console.Testing/TestConsoleInput.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,21 @@ public bool IsKeyAvailable()
}

/// <inheritdoc/>
public ConsoleKeyInfo? ReadKey(bool intercept)
public ConsoleKeyInfo ReadKey(bool intercept, CancellationToken cancellationToken = default)
{
if (_input.Count == 0)
{
throw new InvalidOperationException("No input available.");
}

cancellationToken.ThrowIfCancellationRequested();

return _input.Dequeue();
}

/// <inheritdoc/>
public Task<ConsoleKeyInfo?> ReadKeyAsync(bool intercept, CancellationToken cancellationToken)
[Obsolete("This method will be removed in a future release. Use the synchronous ReadKey() method instead.", error: false)]
public Task<ConsoleKeyInfo> ReadKeyAsync(bool intercept, CancellationToken cancellationToken)
{
return Task.FromResult(ReadKey(intercept));
}
Expand Down
20 changes: 12 additions & 8 deletions src/Spectre.Console/AnsiConsole.Prompt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,28 @@ public static partial class AnsiConsole
/// </summary>
/// <typeparam name="T">The prompt result type.</typeparam>
/// <param name="prompt">The prompt to display.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The prompt input result.</returns>
public static T Prompt<T>(IPrompt<T> prompt)
public static T Prompt<T>(IPrompt<T> prompt, CancellationToken cancellationToken = default)
{
if (prompt is null)
{
throw new ArgumentNullException(nameof(prompt));
}

return prompt.Show(Console);
return prompt.Show(Console, cancellationToken);
}

/// <summary>
/// Displays a prompt to the user.
/// </summary>
/// <typeparam name="T">The prompt result type.</typeparam>
/// <param name="prompt">The prompt markup text.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The prompt input result.</returns>
public static T Ask<T>(string prompt)
public static T Ask<T>(string prompt, CancellationToken cancellationToken = default)
{
return new TextPrompt<T>(prompt).Show(Console);
return new TextPrompt<T>(prompt).Show(Console, cancellationToken);
}

/// <summary>
Expand All @@ -38,26 +40,28 @@ public static T Ask<T>(string prompt)
/// <typeparam name="T">The prompt result type.</typeparam>
/// <param name="prompt">The prompt markup text.</param>
/// <param name="defaultValue">The default value.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The prompt input result.</returns>
public static T Ask<T>(string prompt, T defaultValue)
public static T Ask<T>(string prompt, T defaultValue, CancellationToken cancellationToken = default)
{
return new TextPrompt<T>(prompt)
.DefaultValue(defaultValue)
.Show(Console);
.Show(Console, cancellationToken);
}

/// <summary>
/// Displays a prompt with two choices, yes or no.
/// </summary>
/// <param name="prompt">The prompt markup text.</param>
/// <param name="defaultValue">Specifies the default answer.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns><c>true</c> if the user selected "yes", otherwise <c>false</c>.</returns>
public static bool Confirm(string prompt, bool defaultValue = true)
public static bool Confirm(string prompt, bool defaultValue = true, CancellationToken cancellationToken = default)
{
return new ConfirmationPrompt(prompt)
{
DefaultValue = defaultValue,
}
.Show(Console);
.Show(Console, cancellationToken);
}
}
11 changes: 2 additions & 9 deletions src/Spectre.Console/Extensions/AnsiConsoleExtensions.Input.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Spectre.Console;
/// </summary>
public static partial class AnsiConsoleExtensions
{
internal static async Task<string> ReadLine(this IAnsiConsole console, Style? style, bool secret, char? mask, IEnumerable<string>? items = null, CancellationToken cancellationToken = default)
internal static string ReadLine(this IAnsiConsole console, Style? style, bool secret, char? mask, IEnumerable<string>? items = null, CancellationToken cancellationToken = default)
{
if (console is null)
{
Expand All @@ -19,14 +19,7 @@ internal static async Task<string> ReadLine(this IAnsiConsole console, Style? st

while (true)
{
cancellationToken.ThrowIfCancellationRequested();
var rawKey = await console.Input.ReadKeyAsync(true, cancellationToken).ConfigureAwait(false);
if (rawKey == null)
{
continue;
}

var key = rawKey.Value;
var key = console.Input.ReadKey(true, cancellationToken);
if (key.Key == ConsoleKey.Enter)
{
return text;
Expand Down
22 changes: 13 additions & 9 deletions src/Spectre.Console/Extensions/AnsiConsoleExtensions.Prompt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@ public static partial class AnsiConsoleExtensions
/// <typeparam name="T">The prompt result type.</typeparam>
/// <param name="console">The console.</param>
/// <param name="prompt">The prompt to display.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The prompt input result.</returns>
public static T Prompt<T>(this IAnsiConsole console, IPrompt<T> prompt)
{
public static T Prompt<T>(this IAnsiConsole console, IPrompt<T> prompt, CancellationToken cancellationToken = default)
{
if (prompt is null)
{
throw new ArgumentNullException(nameof(prompt));
}

return prompt.Show(console);
return prompt.Show(console, cancellationToken);
}

/// <summary>
Expand All @@ -28,10 +29,11 @@ public static T Prompt<T>(this IAnsiConsole console, IPrompt<T> prompt)
/// <typeparam name="T">The prompt result type.</typeparam>
/// <param name="console">The console.</param>
/// <param name="prompt">The prompt markup text.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The prompt input result.</returns>
public static T Ask<T>(this IAnsiConsole console, string prompt)
public static T Ask<T>(this IAnsiConsole console, string prompt, CancellationToken cancellationToken = default)
{
return new TextPrompt<T>(prompt).Show(console);
return new TextPrompt<T>(prompt).Show(console, cancellationToken);
}

/// <summary>
Expand All @@ -41,12 +43,13 @@ public static T Ask<T>(this IAnsiConsole console, string prompt)
/// <param name="console">The console.</param>
/// <param name="prompt">The prompt markup text.</param>
/// <param name="culture">Specific CultureInfo to use when converting input.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The prompt input result.</returns>
public static T Ask<T>(this IAnsiConsole console, string prompt, CultureInfo? culture)
public static T Ask<T>(this IAnsiConsole console, string prompt, CultureInfo? culture, CancellationToken cancellationToken = default)
{
var textPrompt = new TextPrompt<T>(prompt);
textPrompt.Culture = culture;
return textPrompt.Show(console);
return textPrompt.Show(console, cancellationToken);
}

/// <summary>
Expand All @@ -55,13 +58,14 @@ public static T Ask<T>(this IAnsiConsole console, string prompt, CultureInfo? cu
/// <param name="console">The console.</param>
/// <param name="prompt">The prompt markup text.</param>
/// <param name="defaultValue">Specifies the default answer.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns><c>true</c> if the user selected "yes", otherwise <c>false</c>.</returns>
public static bool Confirm(this IAnsiConsole console, string prompt, bool defaultValue = true)
public static bool Confirm(this IAnsiConsole console, string prompt, bool defaultValue = true, CancellationToken cancellationToken = default)
{
return new ConfirmationPrompt(prompt)
{
DefaultValue = defaultValue,
}
.Show(console);
.Show(console, cancellationToken);
}
}
16 changes: 12 additions & 4 deletions src/Spectre.Console/IAnsiConsoleInput.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,23 @@ public interface IAnsiConsoleInput
/// <summary>
/// Reads a key from the console.
/// </summary>
/// <param name="intercept">Whether or not to intercept the key.</param>
/// <param name="intercept">
/// Determines whether to display the pressed key in the console window.
/// <see langword="true"/> to not display the pressed key; otherwise, <see langword="false"/>.
/// </param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The key that was read.</returns>
ConsoleKeyInfo? ReadKey(bool intercept);
ConsoleKeyInfo ReadKey(bool intercept, CancellationToken cancellationToken = default);

/// <summary>
/// Reads a key from the console.
/// </summary>
/// <param name="intercept">Whether or not to intercept the key.</param>
/// <param name="intercept">
/// Determines whether to display the pressed key in the console window.
/// <see langword="true"/> to not display the pressed key; otherwise, <see langword="false"/>.
/// </param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The key that was read.</returns>
Task<ConsoleKeyInfo?> ReadKeyAsync(bool intercept, CancellationToken cancellationToken);
[Obsolete("This method will be removed in a future release. Use the synchronous ReadKey() method instead.", error: false)]
Task<ConsoleKeyInfo> ReadKeyAsync(bool intercept, CancellationToken cancellationToken);
}
34 changes: 9 additions & 25 deletions src/Spectre.Console/Internal/DefaultInput.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,38 +19,22 @@ public bool IsKeyAvailable()
return System.Console.KeyAvailable;
}

public ConsoleKeyInfo? ReadKey(bool intercept)
public ConsoleKeyInfo ReadKey(bool intercept, CancellationToken cancellationToken)
{
if (!_profile.Capabilities.Interactive)
cancellationToken.ThrowIfCancellationRequested();

while (!IsKeyAvailable())
{
throw new InvalidOperationException("Failed to read input in non-interactive mode.");
cancellationToken.ThrowIfCancellationRequested();
Thread.Sleep(5);
}

return System.Console.ReadKey(intercept);
}

public async Task<ConsoleKeyInfo?> ReadKeyAsync(bool intercept, CancellationToken cancellationToken)
[Obsolete("This method will be removed in a future release. Use the synchronous ReadKey() method instead.", error: true)]
public Task<ConsoleKeyInfo> ReadKeyAsync(bool intercept, CancellationToken cancellationToken)
{
if (!_profile.Capabilities.Interactive)
{
throw new InvalidOperationException("Failed to read input in non-interactive mode.");
}

while (true)
{
if (cancellationToken.IsCancellationRequested)
{
return null;
}

if (System.Console.KeyAvailable)
{
break;
}

await Task.Delay(5, cancellationToken).ConfigureAwait(false);
}

return ReadKey(intercept);
return Task.FromResult(ReadKey(intercept, cancellationToken));
}
}
17 changes: 9 additions & 8 deletions src/Spectre.Console/Prompts/ConfirmationPrompt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,7 @@ public ConfirmationPrompt(string prompt)
}

/// <inheritdoc/>
public bool Show(IAnsiConsole console)
{
return ShowAsync(console, CancellationToken.None).GetAwaiter().GetResult();
}

/// <inheritdoc/>
public async Task<bool> ShowAsync(IAnsiConsole console, CancellationToken cancellationToken)
public bool Show(IAnsiConsole console, CancellationToken cancellationToken = default)
{
var comparer = Comparer ?? StringComparer.CurrentCultureIgnoreCase;

Expand All @@ -89,8 +83,15 @@ public async Task<bool> ShowAsync(IAnsiConsole console, CancellationToken cancel
.AddChoice(Yes)
.AddChoice(No);

var result = await prompt.ShowAsync(console, cancellationToken).ConfigureAwait(false);
var result = prompt.Show(console, cancellationToken);

return comparer.Compare(Yes.ToString(), result.ToString()) == 0;
}

/// <inheritdoc/>
[Obsolete("This method will be removed in a future release. Use the synchronous Show() method instead.", error: false)]
public Task<bool> ShowAsync(IAnsiConsole console, CancellationToken cancellationToken)
{
return Task.FromResult(Show(console, cancellationToken));
}
}
4 changes: 3 additions & 1 deletion src/Spectre.Console/Prompts/IPrompt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ public interface IPrompt<T>
/// Shows the prompt.
/// </summary>
/// <param name="console">The console.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The prompt input result.</returns>
T Show(IAnsiConsole console);
T Show(IAnsiConsole console, CancellationToken cancellationToken = default);

/// <summary>
/// Shows the prompt asynchronously.
/// </summary>
/// <param name="console">The console.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>The prompt input result.</returns>
[Obsolete("This method will be removed in a future release. Use the synchronous Show() method instead.", error: false)]
Task<T> ShowAsync(IAnsiConsole console, CancellationToken cancellationToken);
}
11 changes: 2 additions & 9 deletions src/Spectre.Console/Prompts/List/ListPrompt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public ListPrompt(IAnsiConsole console, IListPromptStrategy<T> strategy)
_strategy = strategy ?? throw new ArgumentNullException(nameof(strategy));
}

public async Task<ListPromptState<T>> Show(
public ListPromptState<T> Show(
ListPromptTree<T> tree,
Func<T, string> converter,
SelectionMode selectionMode,
Expand Down Expand Up @@ -52,14 +52,7 @@ public async Task<ListPromptState<T>> Show(

while (true)
{
cancellationToken.ThrowIfCancellationRequested();
var rawKey = await _console.Input.ReadKeyAsync(true, cancellationToken).ConfigureAwait(false);
if (rawKey == null)
{
continue;
}

var key = rawKey.Value;
var key = _console.Input.ReadKey(true, cancellationToken);
var result = _strategy.HandleInput(key, state);
if (result == ListPromptInputResult.Submit)
{
Expand Down
17 changes: 9 additions & 8 deletions src/Spectre.Console/Prompts/MultiSelectionPrompt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,12 @@ public IMultiSelectionItem<T> AddChoice(T item)
}

/// <inheritdoc/>
public List<T> Show(IAnsiConsole console)
{
return ShowAsync(console, CancellationToken.None).GetAwaiter().GetResult();
}

/// <inheritdoc/>
public async Task<List<T>> ShowAsync(IAnsiConsole console, CancellationToken cancellationToken)
public List<T> Show(IAnsiConsole console, CancellationToken cancellationToken = default)
{
// Create the list prompt
var prompt = new ListPrompt<T>(console, this);
var converter = Converter ?? TypeConverterHelper.ConvertToString;
var result = await prompt.Show(Tree, converter, Mode, false, false, PageSize, WrapAround, cancellationToken).ConfigureAwait(false);
var result = prompt.Show(Tree, converter, Mode, false, false, PageSize, WrapAround, cancellationToken);

if (Mode == SelectionMode.Leaf)
{
Expand All @@ -111,6 +105,13 @@ public async Task<List<T>> ShowAsync(IAnsiConsole console, CancellationToken can
.ToList();
}

/// <inheritdoc/>
[Obsolete("This method will be removed in a future release. Use the synchronous Show() method instead.", error: false)]
public Task<List<T>> ShowAsync(IAnsiConsole console, CancellationToken cancellationToken)
{
return Task.FromResult(Show(console, cancellationToken));
}

/// <summary>
/// Returns all parent items of the given <paramref name="item"/>.
/// </summary>
Expand Down
Loading

0 comments on commit 961af98

Please sign in to comment.