Refactor prompt options and forms to use PromptConfiguration for improved extensibility#332
Refactor prompt options and forms to use PromptConfiguration for improved extensibility#332
Conversation
…oved extensibility
There was a problem hiding this comment.
Pull request overview
This PR refactors Sharprompt’s form and options infrastructure to centralize configuration (colors/symbols/driver factory) via PromptConfiguration, reduce duplication across selection-based forms, and tighten resource ownership around the console driver lifecycle.
Changes:
- Introduces
PromptConfigurationand updates forms/rendering/buffer code to consistently use it (symbols/colors, cancel behavior, driver factory). - Adds a shared
PromptOptionsbase type for common option fields/validation and updates existing option types to inherit from it. - Refactors
SelectFormandMultiSelectFormto share pagination/filter/key handling via a newSelectFormBase<TItem, TResult>.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Sharprompt/SelectOptions.cs | Inherits from PromptOptions; validation now calls base.EnsureOptions(). |
| Sharprompt/PromptOptions.cs | Adds shared Message + EnsureOptions() for option validation. |
| Sharprompt/PromptConfiguration.cs | Adds configuration object for driver factory, symbols, and color schema. |
| Sharprompt/Prompt.Configuration.cs | Routes existing Prompt.* configuration APIs through PromptConfiguration. |
| Sharprompt/Prompt.Basic.cs | Passes shared configuration into forms to ensure consistent rendering/behavior. |
| Sharprompt/PasswordOptions.cs | Inherits from PromptOptions; validation calls base.EnsureOptions(). |
| Sharprompt/MultiSelectOptions.cs | Inherits from PromptOptions; validation calls base.EnsureOptions(). |
| Sharprompt/ListOptions.cs | Inherits from PromptOptions; validation calls base.EnsureOptions(). |
| Sharprompt/Internal/OffscreenBufferExtensions.cs | Uses OffscreenBuffer.Configuration for symbols/colors instead of static Prompt.*. |
| Sharprompt/Internal/OffscreenBuffer.cs | Takes PromptConfiguration; no longer owns/implements disposal for the console driver. |
| Sharprompt/InputOptions.cs | Inherits from PromptOptions and removes redundant EnsureOptions() implementation. |
| Sharprompt/Forms/TextFormBase.cs | Requires PromptConfiguration and forwards it to FormBase. |
| Sharprompt/Forms/SelectFormBase.cs | New shared base class for selection-form paging/filtering and key handling. |
| Sharprompt/Forms/SelectForm.cs | Refactored to inherit from SelectFormBase and use Configuration for symbols. |
| Sharprompt/Forms/PasswordForm.cs | Constructor updated to accept PromptConfiguration. |
| Sharprompt/Forms/MultiSelectForm.cs | Refactored to inherit from SelectFormBase and use Configuration for symbols. |
| Sharprompt/Forms/ListForm.cs | Constructor updated to accept PromptConfiguration. |
| Sharprompt/Forms/InputForm.cs | Constructor updated to accept PromptConfiguration. |
| Sharprompt/Forms/FormRenderer.cs | Now constructed with configuration; no longer disposable (lifetime moved to driver). |
| Sharprompt/Forms/FormBase.cs | Accepts PromptConfiguration, constructs renderer with it, and disposes the console driver. |
| Sharprompt/Forms/ConfirmForm.cs | Constructor updated to accept PromptConfiguration. |
| Sharprompt/ConfirmOptions.cs | Inherits from PromptOptions and removes redundant EnsureOptions(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| protected FormBase() | ||
| protected FormBase(PromptConfiguration configuration) | ||
| { |
There was a problem hiding this comment.
FormBase stores and uses the passed PromptConfiguration but doesn't validate it for null. If a null configuration is ever passed, this will result in a NullReferenceException when accessing configuration.ConsoleDriverFactory() or later when reading _configuration.ThrowExceptionOnCancel. Consider adding ArgumentNullException.ThrowIfNull(configuration); at the start of the constructor to fail fast with a clear exception.
| { | |
| { | |
| ArgumentNullException.ThrowIfNull(configuration); |
…#332) with ConsoleKeyBinding architecture Co-authored-by: shibayan <1356444+shibayan@users.noreply.github.com>
…ed handlers, ESC cancellation, fixed driver lifetime (#331) * Initial plan * Initial plan Co-authored-by: shibayan <1356444+shibayan@users.noreply.github.com> * Refactor overall architecture: ConsoleKeyBinding, simplified handlers, ESC key, fixed lifecycle Co-authored-by: shibayan <1356444+shibayan@users.noreply.github.com> * Resolve conflicts with master: integrate PromptConfiguration refactor (#332) with ConsoleKeyBinding architecture Co-authored-by: shibayan <1356444+shibayan@users.noreply.github.com> * Delete global.json --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: shibayan <1356444+shibayan@users.noreply.github.com> Co-authored-by: Tatsuro Shibamura <me@shibayan.jp>
This pull request refactors the prompt form classes to improve code reuse and configuration handling, especially for selection-based prompts. The changes introduce a new abstract base class for selection forms, streamline paginator and key handler management, and ensure that prompt configuration is consistently passed throughout the forms. Additionally, resource disposal is improved to prevent leaks.
Refactoring and abstraction of selection forms:
SelectFormBase<TItem, TResult>to encapsulate common logic for selection-based forms (SelectFormandMultiSelectForm), including paginator initialization, key handler mapping, text input handling, and pagination rendering. (Sharprompt/Forms/SelectFormBase.cs)SelectForm<T>andMultiSelectForm<T>to inherit fromSelectFormBase, removing duplicated paginator and key handler logic, and delegating shared functionality to the base class. (Sharprompt/Forms/SelectForm.cs,Sharprompt/Forms/MultiSelectForm.cs) [1] [2]Prompt configuration propagation:
PromptConfigurationparameter and consistently use it for rendering and symbol selection. This ensures that all forms are properly configured and customizable. (Sharprompt/Forms/FormBase.cs,Sharprompt/Forms/ConfirmForm.cs,Sharprompt/Forms/InputForm.cs,Sharprompt/Forms/ListForm.cs,Sharprompt/Forms/PasswordForm.cs,Sharprompt/Forms/SelectForm.cs,Sharprompt/Forms/MultiSelectForm.cs) [1] [2] [3] [4] [5] [6] [7]Resource management improvements:
FormBaseand related classes to ensure that the console driver is disposed rather than just the renderer, addressing potential resource leaks. (Sharprompt/Forms/FormBase.cs,Sharprompt/Forms/FormRenderer.cs) [1] [2]Options and validation updates:
ConfirmOptionsandInputOptions<T>to inherit fromPromptOptionsand removed redundantEnsureOptionsmethods, simplifying options validation and inheritance. (Sharprompt/ConfirmOptions.cs,Sharprompt/InputOptions.cs) [1] [2]Rendering and buffer improvements:
OffscreenBufferto acceptPromptConfigurationfor consistent rendering and symbol usage across all prompts. (Sharprompt/Internal/OffscreenBuffer.cs)