Fix unintended initial selection for value-type items and add form interaction tests#374
Merged
Merged
Conversation
…enerator The v3.1 source generator only sees enum declarations in the consuming compilation, so Prompt.Select/MultiSelect with a BCL enum or an enum from an assembly built without the analyzer threw ArgumentNullException (Items). Restore the v3.0 behavior by generating enum metadata at runtime via reflection when no generated registration exists, honoring the same Display(Name/Order) semantics as the generator. Generated registrations still take precedence, keeping the AOT-friendly path intact. Fixes #357 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…teraction tests Paginator.UpdatePageSize ignored the TryGetSelectedItem result and reused the out parameter, so for value-type items (e.g. enum Select) the item equal to default(T) was silently pre-selected on the first render. It also compared the requested page size against the clamped stored value, causing a full reinitialization on every render whenever the item count was smaller than the requested size. Add a scriptable FakeConsoleDriver and interaction tests that drive the Select/MultiSelect/Input/Confirm forms with queued key input, covering arrow-key selection, filtering, validation errors, default values, Escape cancellation, and the enum fallback end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes two paginator regressions that affected selection behavior and render-time reinitialization, and adds end-to-end form interaction tests using a scriptable console driver to prevent similar issues going forward.
Changes:
- Fix
Paginator<T>.UpdatePageSizeso value-type items don’t become implicitly “selected” viadefault(T)when no selection exists. - Fix
UpdatePageSizeearly-return logic by comparing against the clamped/effective page size to avoid unnecessary reinitialization (e.g.,PageSize = int.MaxValuecases). - Add a
FakeConsoleDriverand new interaction tests covering selection, filtering, validation, defaults, cancellation, and enum fallback.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Sharprompt/Internal/Paginator.cs | Fixes selection preservation and page-size comparison logic in UpdatePageSize. |
| tests/Sharprompt.Tests/Internal/PaginatorTests.cs | Adds unit tests covering both paginator regressions. |
| tests/Sharprompt.Tests/Forms/FakeConsoleDriver.cs | Adds a scriptable console driver to enable deterministic end-to-end form interaction tests. |
| tests/Sharprompt.Tests/Forms/FormInteractionTests.cs | Adds end-to-end tests for Select/MultiSelect/Input/Confirm flows and cancellation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Write now advances CursorTop when text exceeds BufferWidth, using the same East Asian width calculation as OffscreenBuffer and matching the deferred wrap behavior its cursor math assumes. Add a test with a message longer than the buffer width to exercise the wrapping path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…om/shibayan/Sharprompt into fix/paginator-value-type-selection
…ution EnumMetadataRegistry is a global static, so registering metadata for string leaked an uppercase TextSelector into the form interaction tests that use SelectOptions<string>, failing depending on test execution order. Register a type unique to the test instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 5, 2026
This was referenced Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two changes in
Paginator<T>.UpdatePageSize:TryGetSelectedItemand reused theoutparameter, so for value-type items (e.g. enumSelect) the item equal todefault(T)was silently pre-selected on the first render — inconsistent with reference-type items, which start unselected.PageSize = int.MaxValuedefault), the early return never hit and the paginator was fully reinitialized on every render.This PR also adds a form interaction test harness: a scriptable
FakeConsoleDriverthat feeds queued key input throughIConsoleDriver, plus tests drivingSelectForm/MultiSelectForm/InputForm/ConfirmFormend to end (arrow-key selection, filtering, validation errors, default values, Escape cancellation, and the enum fallback). These tests are what surfaced theUpdatePageSizebug.Notes
Stacked on #372 — the end-to-end enum test requires the reflection fallback. Merge #372 first; this PR targets its branch and should retarget to
masteronce it lands.Test plan
Paginatorunit tests covering both regressions🤖 Generated with Claude Code