Add sidebar.width control command for the sidebar divider position - #512
Merged
Conversation
The sidebar divider was the only persisted per-window state with no control command and no read-back: its sole writer was the drag gesture in WindowContentView. That matters beyond tidiness, because terminal width is the window width minus the sidebar width minus the divider, so the sidebar sets a floor on reachable columns that no command could lower. A caller wanting fewer columns than the 640pt window minimum allows had no way to get them. agtermctl sidebar width <points> [--window W] Clamps to the existing 160...560pt drag bounds and echoes the STORED width, so an out-of-range request is distinguishable from an honored one - both answer ok otherwise. The echo prints exactly rather than rounding: a caller comparing its request against the echo would read a rounded 271.34 as a clamp that never happened. Points are Double because the drag writes a fractional cursor x, so an Int could not express every width the GUI can reach. Read-back is the tree's top-level sidebarWidth and deliberately not window.list: nothing needs width discovery across windows, which is all the cached copy would add. Takes --window like sidebar.expand rather than being frontmost-only like sidebar and sidebar.mode. ControlActions defaults the new arm in its public extension rather than making it a bare requirement, so a shared-dispatcher host outside this repo owes no conformance change. That does not make such a host build unchanged - adding a Command case breaks any exhaustive switch over it first, which is true of every new command. Two supporting moves, both forced by file length limits: - AppStore+Sidebar.swift takes the new setter plus clampSidebarWidth, which replaces the clamp expression that restore() and the drag handler each spelled separately. The split divider already had clampSplitRatio; this closes the asymmetry. - ControlProjection.swift takes the read-back projection types (tree and window nodes) out of ControlProtocol.swift, which was at 989 of its 1000-line limit before this change. Pure move, verified byte-identical. Related to #510 and discussion #511.
…ract Review follow-ups on the sidebar.width commit, none of them behavioral. Three AppStore comments described sidebarWidth as drag-driven and its bounds as shared by the drag and restore(). Both were true until setSidebarWidth became a third writer and a third user of those bounds - and the only scriptable one, saving per call where the drag saves once on release. sidebarWidth was missing from two field enumerations that read as exhaustive: the bundled skill's tree top-level list, which is what an installed agent consults to learn what tree returns, and reference.md's tree-only versus both-surfaces split, which contradicted the "fourteen fields" count in the same file. AppStore+Sidebar.swift used `public extension` where all fifteen sibling AppStore+*.swift files declare a plain extension and mark each member public. Dropping the modifier alone would have made both members internal and broken the app target's calls; they are now explicitly public, which also stops a future helper inheriting public by accident. The docs claimed the CLI prints the echoed width "exactly", which a Double cannot promise: the value is Swift's own description, so 300 comes back as "300.0" and 300.00, 3e2 and +300 all normalize differently. A caller told to compare its request against the echo to detect a clamp would read that as a clamp that never happened - the same failure dropping the rounding removed, arriving from the other side. The claim is now that the value is preserved without fixed-decimal rounding, and the comparison the docs ask for is numeric rather than string equality. No formatting code changed.
Master gained the persistent-sessions/zmx work and session.swap, another new control command in the same seams as sidebar.width. Two files conflicted and both were resolved master-first, re-applying this branch's additions on top rather than resolving hunk by hunk, which would have dropped master's edits to the types this branch moves. ControlProtocol.swift: took master's file, re-added the Command case, the ControlArgs field and the ControlResult field with their initializer wiring, then redid the projection split against master's current block. The moved declarations are byte-identical to master's, so backedByZmx, splitCommandWait and the changed initializers travel with them. ControlProjection.swift is rebuilt from that block rather than patched, and its header now names ControlResponse.swift, where master moved the response envelope. ControlDispatcher.swift: took master's file and re-added only the protocol requirement, the app-command classification and the dispatch arm, preserving swapSessionPanes and the restore and zmx methods. The default implementation moves into master's new ControlActionsDefaults.swift instead of an inline extension. It keeps its own refusal string rather than the shared ControlActionsUnsupported helper, which says "on this platform": a sidebar has a divider on any platform, so a host refusing this has not implemented the command rather than lacking the thing it moves. ControlDispatcherSidebarTests.swift takes this branch's dispatcher cases out of ControlDispatcherTests.swift, which master's own additions plus these pushed past the 2000-line test limit. It joins the Dashboard, Hud, Workspace, Overlay and Pick files already split off that suite. Gates on the merged tree: swift test 2795 in 112 suites, make test-app 368 with 2 skipped, make lint clean, plus the targeted hosted and end-to-end sidebar cases.
…ments Review follow-ups on the merged branch. One test, three comments, no behavior change. The formatter tests pinned 271.3 and 271.34, both fractional, while three surfaces now tell callers that 300 comes back as "300.0" and to compare numerically because of it. Trimming the trailing .0 would have kept both tests green and made all three wrong, including the advice a caller's clamp-detection script is written against. The three cases are now one parameterized test covering both directions; mutation-checked by adding the trim, which fails the integral case alone. clampSidebarWidth's doc justified its precondition by naming callers and listed two, omitting restore() - the one whose input is least obviously finite, a Double decoded from a snapshot the comment above it calls hand-editable. It now states the contract instead of enumerating call sites, so a fourth caller cannot make it wrong again. The two AppStore comments rewritten last round ran 169 and 138 columns, the longest in that file, because the file sits at exactly its 1000-line limit and wrapping them costs lines it does not have. Shorter text fits 89 and 92 columns while still naming the control writer. ControlProjection.swift's header was a `///` block followed by a blank line, so it documented no declaration - the only instance of that shape in the codebase. ControlActionsDefaults.swift and ControlPayloads.swift, both split out under the same line limit, use plain `//`.
Deploying agterm with
|
| Latest commit: |
1d8ee79
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://214d148c.agterm.pages.dev |
| Branch Preview URL: | https://sidebar-width-command.agterm.pages.dev |
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.
sidebar width is persisted per window and adjustable by dragging, but it had no control command or read-back. Dragging was the only user-facing way to change it.
that matters beyond tidiness. Terminal width is the window width minus the sidebar width minus a 1pt divider, and the window has a hard 640pt minimum (
agtermApp.swift:127) thatwindow resizesilently clamps to. With the sidebar visible, a controller could not reach a terminal width below the floor set by that minimum and the current sidebar width.clamps to the existing 160...560pt drag bounds and echoes the stored width. Without the echo a clamped request and an honored one both answer bare ok. The echo is the stored value, not a measured on-screen width, and the CLI preserves it without fixed-decimal rounding: rounding an honored 271.34 to 271.3 would report a clamp that never happened. Points are
Doublebecause the drag writes a fractional cursor x, so anIntcould not express every width the GUI can reach. The comparison callers are told to make is numeric, since the echo is a double and300comes back300.0.read-back is the tree's top-level
sidebarWidth, deliberately notwindow list. No caller needs width discovery across windows, which is all awindow listcopy would add. Takes--windowlikesidebar expandrather than being frontmost-only likesidebarandsidebar mode.ControlActions.setSidebarWidthhas a default unsupported response, so existing conformers need no new method. A downstream can still need source changes for exhaustiveCommandswitches, as with any added command.two supporting moves, both forced by file length limits
AppStore+Sidebar.swifttakes the new setter plusclampSidebarWidth, which replaces a clamp expression thatrestore()and the drag handler each spelled separately. The split divider already hadclampSplitRatio, so this closes the asymmetry.ControlProjection.swiftmoves the read-back projection types out ofControlProtocol.swift, which was at 989 of its 1000-line limit before this change.coverage
dispatcher routing with and without
--window, missing and non-finite points, low and high clamps, no-op write suppression, tree population, nil omission for both the tree field and the args field, CLI parse and plain output. Five hosted cases inControlServerSidebarWidthTestsincluding a background--windowwrite that asserts the frontmost store is untouched, and one end-to-end case over the launched app's real socket covering set, echo, tree read-back, clamp echo and the missing-argument refusal.swift test2671 in 101 suites,make test-app271,make lintclean,xcodebuild -scheme agterm -configuration Debug buildsucceeds.Fix #510. Related to discussion #511.