⌃F gives its results the room the terminal has - #36
Merged
Conversation
Three defects, all visible in one reported frame: the results were squeezed into twenty-odd cells while a sixty-cell window column of blank sat beside them. `_lines` is wider than the workspace on purpose — RestorePreviousSession buffers a restore log the workspace cannot place under its own id, so its pane refills if that channel speaks again — and those ids have no WorkspaceWindow to be titled from, so WindowTitle handed back the raw `spawn:24:World|Character:Target` id. Every row was padded to it. Those rows were dead as well: ActivateWindow refuses a window no pane holds, so ⏎ inserted its bar into a buffer nothing paints. The corpus is now Workspace.WindowsFor's rule, and GoToSearchHit honours Activate's answer. The window column is bounded at eighteen cells and elided past it: a title is not this client's text to trust, and every row pads to the widest one. It also pads by visible width now, so a window called `[Chat]` no longer sits a cell adrift of every other row. And the surface opens at the room the terminal has. It has no unfiltered list to size to — an empty query matches nothing — so the width was measured against the only content an empty surface has, which is its own footer: eighty-odd cells on any terminal, with every result elided to fit a window sized by a key hint. The height always took the desktop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughSearch now returns only pane-held windows, preserves activation validation, and excludes undisplayable buffers. Search labels are bounded and aligned. Search surfaces use the available terminal width. End-to-end tests cover these behaviors. ChangesSearch behavior and presentation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change improves search-result layout and excludes windows that cannot be displayed or activated. No actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Operator
participant SearchPrompt
participant SharpMUTermApp
participant WorkspaceLayout
Operator->>SearchPrompt: enter search query
SearchPrompt->>SharpMUTermApp: request matching windows
SharpMUTermApp->>WorkspaceLayout: filter windows by pane placement
WorkspaceLayout-->>SharpMUTermApp: pane-held matches
SharpMUTermApp-->>SearchPrompt: render matching results
Operator->>SearchPrompt: select result
SearchPrompt->>SharpMUTermApp: activate target window
SharpMUTermApp->>WorkspaceLayout: validate placement and activation
WorkspaceLayout-->>SharpMUTermApp: activation result
``
<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->
<details>
<summary>🚥 Pre-merge checks | ✅ 5</summary>
<details>
<summary>✅ Passed checks (5 passed)</summary>
| Check name | Status | Explanation |
| :------------------------: | :------- | :-------------------------------------------------------------------------------------------------- |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly describes the main change: the search results use the terminal's available width. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
</details>
</details>
<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->
<details>
<summary>✨ Finishing Touches</summary>
<details>
<summary>📝 Generate docstrings</summary>
- [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> Create stacked PR
- [ ] <!-- {"checkboxId": "3e1879ae-f29b-4d0d-8e06-d12b7ba33d98"} --> Commit on current branch
</details>
</details>
<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->
---
<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>
<!-- tips_end -->
|
There was a problem hiding this comment.
Pull request overview
This PR fixes the widened-scope (⌥A) ⌃F search UI so results use the available terminal width, avoid including buffers from windows that aren’t placed in any pane, and ensure the window-label column can’t dominate the layout.
Changes:
- Filter the search corpus to only windows currently placed in the workspace layout, and refuse navigation when activation fails.
- Size the search surface to the terminal’s available width (not the footer’s width), and cap/elide the window-label column with correct visible-width padding.
- Add end-to-end and prompt-level tests that pin corpus selection, surface sizing, label elision, and bracketed-label alignment.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/SharpMUTerm.Tui.Tests/SpawnWindowIdUpgradeTests.cs | Adds a regression test ensuring buffered-but-unplaced windows are excluded from ⌃F search results. |
| tests/SharpMUTerm.Tui.Tests/SearchPromptTests.cs | Pins max label-column width, elision behavior, and correct padding for escaped bracket labels. |
| tests/SharpMUTerm.Tui.Tests/SearchEndToEndTests.cs | Adds a rendered-frame assertion that the surface opens to the terminal’s available width. |
| src/SharpMUTerm.Tui/SharpMUTermApp.cs | Filters searchable windows to those placed in panes and makes “go to hit” honor activation failure. |
| src/SharpMUTerm.Tui/SearchSurface.cs | Updates initial sizing so the surface uses available desktop width rather than measuring footer content. |
| src/SharpMUTerm.Tui/SearchPrompt.cs | Introduces MaxLabelWidth, label elision, and visible-width padding for aligned columns. |
| CLAUDE.md | Documents the new ⌃F behavior/constraints and rationale in the project brief. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
The report was a frame: with the scope widened to every window, the matched
lines were squeezed into about twenty cells while a sixty-cell column of
mostly-blank window names sat beside them. Measured off the screenshot, the
window column was 56 cells wide and the result text got 23 on a 166-column
terminal — and the surface itself was only 86 cells of that terminal.
Three separate defects, all in the ⌥A path.
The corpus held windows no pane holds
_linesis wider than the workspace on purpose:RestorePreviousSessionbuffers a restore log the workspace cannot place under its own id, so that
pane refills if the channel ever speaks again. Such an id has no
WorkspaceWindowto be titled from, soWindowTitlehanded back the rawspawn:24:World|Character:Targetstring — and every row in the list is paddedto the widest label, so one of those spent sixty cells of every row.
Those rows were dead as well.
Workspace.ActivateWindowrefuses a window nopane holds, and
GoToSearchHitignored the answer: it inserted its bar into abuffer nothing paints, set
_searchMarkon it, and scrolled a pane that doesnot exist. Silently.
The corpus is now
Workspace.WindowsFor's rule — placed windows only — andGoToSearchHithonoursActivate, refusing out loud instead. Two lines ofdefence rather than one.
The window column was unbounded
A title is not this client's text to trust (
Snippetcaps it at sixty cells,and a window can be titled from a world's own output), and every row pads to
the widest one.
SearchPrompt.MaxLabelWidthbounds it at eighteen and elidespast it — the rail's rule, one surface over.
It also pads by visible width now. A window may be called
[Chat], escapingturns each bracket into two characters standing for one cell, and
PadRighttherefore came up a cell short per bracket and left that row's text adrift of
every other row's.
The surface was sized to its own footer
It has no unfiltered list to size to — an empty query matches nothing — so the
width was measured against the only content an empty surface has, which is its
key hints: eighty-odd cells on any terminal, with every result then elided to
fit a window sized by a hint. The height has always taken the room there is.
Both dimensions do now. On a 166-column terminal that is 160 cells instead of
86.
Verification
dotnet build -c Release SharpMUTerm.slnxclean and warning-free; all fivesuites green — Core 938, Graphics 83, Scripting 42, Web 37, Tui 1806, zero
failures.
New pins:
ABufferedWindowNoPaneHoldsIsNotSearched— driven through the real producerof the state (a restore log the workspace cannot place), and it checks the
placed window's restored lines are still found, so an empty result for the
other one is the pane rule at work rather than a search finding nothing.
TheSurfaceOpensAtTheRoomTheTerminalHasRatherThanAtItsFootersWidth— read offa rendered frame, not off the sizing expression.
ALongWindowNameIsElidedRatherThanSpendingTheRowOnItselfABracketedWindowNamePadsToTheSameColumnAsEveryOtherRowThe four
search*snapshot views were re-rendered and read back as cell grids.search-allnow spans columns 3–162 of a 166-column terminal with a six-cellwindow column;
search,search-regexandsearch-landedare unchanged inbehaviour.
CLAUDE.md's ⌃F section carries the two new decisions.
🤖 Generated with Claude Code
Summary by CodeRabbit