Skip to content

ui: dedupe pane helpers, compute commonNames once per frame, show q in footer#6

Merged
rjayasin merged 5 commits into
mainfrom
claude/ui-dedupe
Jun 23, 2026
Merged

ui: dedupe pane helpers, compute commonNames once per frame, show q in footer#6
rjayasin merged 5 commits into
mainfrom
claude/ui-dedupe

Conversation

@rjayasin

@rjayasin rjayasin commented Jun 23, 2026

Copy link
Copy Markdown
Owner

PR 3 of 4 from the review stack. Base: claude/shared-helpers (PR #5) — merge #4 then #5 first; the diff here is only this PR's changes.

Collapses remote/local pane duplication, trims per-frame work, and fixes a footer gap:

  • Generic sort/filter. sortEntries/sortLocalEntries now wrap a generic sortByMode, and filteredEntries/filteredLocalEntries wrap a generic filterByName, so the sort switch and the hidden+substring filter each live in one place.
  • Shared cleanup targets. cleanupTargets and remoteCleanupTargets share computeCleanup, parameterised by base/join/absent for the local-fs vs remote-SFTP cases.
  • commonNames once per frame (perf). It was rebuilt up to four times per render (in displayedEntries, displayedLocalEntries, listLines, localListLines). It's now computed once via frameCommon at the top of the frame and threaded through; the *With variants take the precomputed set, and the public no-arg accessors remain for non-render callers.
  • Footer. The browser footer now ends in q quit like the other panes; helpBookmarks drops its own q quit so footer() adds it consistently (no duplication).

The public sortEntries/filtered*/displayed*/cleanupTargets signatures are unchanged, so the existing UI tests exercise the new shared code paths. go build, go vet, staticcheck -checks=all, and go test -race ./... all pass.

Note on scope: I implemented the pane dedupe at the function level (generics + shared helpers) rather than introducing a single unified pane struct. The remote pane's async, command-driven listing and its selection state make a full struct merge more invasive than valuable — happy to do that deeper refactor as a follow-up if you'd prefer it.

🤖 Generated with Claude Code

Generated by Claude Code

claude added 3 commits June 23, 2026 00:39
@claude
sshx: close the ssh-agent socket after the handshake
5babd50 
authMethods dialed the SSH_AUTH_SOCK unix socket but never closed it, so a
long-running session leaked one descriptor per Dial (plus one more per jump
host). The agent is only consulted during authentication, which ssh.Dial /
NewClientConn complete synchronously before Dial returns, so thread a cleanup
func out through clientConfig and defer it once the handshake is done.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
@claude
sshx: drop dead listing sort from Session.List
2733ef6 
Every caller (connectCmd, listCmd) immediately re-sorts the result with
ui.sortEntries by the user's chosen mode, so the dirs-first sort in List was
never observed. Removing it also deletes the ASCII-only lessFold/toLower
helpers, which disagreed with the Unicode-aware strings.ToLower used by the
UI sort — a latent inconsistency that can no longer surface.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
@claude
sshx: drop always-nil error from PathSize
48a7f72 
PathSize swallowed every walk error and unconditionally returned nil, so the
error return advertised a failure mode that could not happen and forced a dead
err check at the call site. Return just int64, matching its local-filesystem
twin localPathSize.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
@rjayasin rjayasin mentioned this pull request Jun 23, 2026
Merged
claude added 2 commits June 23, 2026 04:13
@claude
dedupe shared helpers into internal/util
f1308ec 
Two helpers were copy-pasted across packages; both now live in a single
internal/util package:

- A byte-count formatter existed as ui.humanSize (int64, "4.2M") and
  update.humanBytes (int, "4.2 MB"). Both are replaced by util.HumanBytes,
  giving one consistent style app-wide. The only visible change is the
  `rtr update` download message, now "4.2M" to match the listing style.
- A "~" home-expansion helper existed three times: sshx.expandHome,
  transfer.expandHome, and ui.expandHomeUI. All three are replaced by
  util.ExpandHome, using the safe variant that returns the path unchanged
  when $HOME is unresolved.

No behavior change beyond the update-message format.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
@claude
ui: dedupe pane helpers, compute commonNames once, show q in footer
2bd4673 
Collapse the remote/local pane duplication and trim per-frame work:

- sortEntries/sortLocalEntries now wrap a generic sortByMode, and
  filteredEntries/filteredLocalEntries wrap a generic filterByName, so the
  sort switch and the hidden+substring filter live in one place each.
- cleanupTargets and remoteCleanupTargets share computeCleanup, parameterised
  by base/join/absent for the local-fs vs remote-SFTP cases.
- commonNames was rebuilt up to four times per render (in displayedEntries,
  displayedLocalEntries, listLines, localListLines). Compute it once via
  frameCommon at the top of the frame and thread it through; the *With
  variants take the precomputed set. The public no-arg accessors remain for
  non-render callers.
- The browser footer now ends in "q quit" like the other panes; helpBookmarks
  drops its own "q quit" so footer adds it consistently (no duplication).

The public sortEntries/filtered*/displayed*/cleanupTargets signatures are
unchanged, so the existing tests exercise the new shared code paths.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LvNkzcutbfXNKAKCSy38H1
@rjayasin
rjayasin force-pushed the claude/shared-helpers branch from 6d101a3 to f1308ec Compare June 23, 2026 04:16
Base automatically changed from claude/shared-helpers to main June 23, 2026 06:33
@rjayasin
rjayasin merged commit 54fa461 into main Jun 23, 2026
1 check passed
@rjayasin
rjayasin deleted the claude/ui-dedupe branch June 23, 2026 21:48
rjayasin added a commit that referenced this pull request Jul 14, 2026
@rjayasin @claude
ui: dedupe pane helpers, compute commonNames once per frame, show q i…
cf8e269 
…n footer (#6)

* sshx: close the ssh-agent socket after the handshake

authMethods dialed the SSH_AUTH_SOCK unix socket but never closed it, so a
long-running session leaked one descriptor per Dial (plus one more per jump
host). The agent is only consulted during authentication, which ssh.Dial /
NewClientConn complete synchronously before Dial returns, so thread a cleanup
func out through clientConfig and defer it once the handshake is done.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* sshx: drop dead listing sort from Session.List

Every caller (connectCmd, listCmd) immediately re-sorts the result with
ui.sortEntries by the user's chosen mode, so the dirs-first sort in List was
never observed. Removing it also deletes the ASCII-only lessFold/toLower
helpers, which disagreed with the Unicode-aware strings.ToLower used by the
UI sort — a latent inconsistency that can no longer surface.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* sshx: drop always-nil error from PathSize

PathSize swallowed every walk error and unconditionally returned nil, so the
error return advertised a failure mode that could not happen and forced a dead
err check at the call site. Return just int64, matching its local-filesystem
twin localPathSize.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* dedupe shared helpers into internal/util

Two helpers were copy-pasted across packages; both now live in a single
internal/util package:

- A byte-count formatter existed as ui.humanSize (int64, "4.2M") and
  update.humanBytes (int, "4.2 MB"). Both are replaced by util.HumanBytes,
  giving one consistent style app-wide. The only visible change is the
  `rtr update` download message, now "4.2M" to match the listing style.
- A "~" home-expansion helper existed three times: sshx.expandHome,
  transfer.expandHome, and ui.expandHomeUI. All three are replaced by
  util.ExpandHome, using the safe variant that returns the path unchanged
  when $HOME is unresolved.

No behavior change beyond the update-message format.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* ui: dedupe pane helpers, compute commonNames once, show q in footer

Collapse the remote/local pane duplication and trim per-frame work:

- sortEntries/sortLocalEntries now wrap a generic sortByMode, and
  filteredEntries/filteredLocalEntries wrap a generic filterByName, so the
  sort switch and the hidden+substring filter live in one place each.
- cleanupTargets and remoteCleanupTargets share computeCleanup, parameterised
  by base/join/absent for the local-fs vs remote-SFTP cases.
- commonNames was rebuilt up to four times per render (in displayedEntries,
  displayedLocalEntries, listLines, localListLines). Compute it once via
  frameCommon at the top of the frame and thread it through; the *With
  variants take the precomputed set. The public no-arg accessors remain for
  non-render callers.
- The browser footer now ends in "q quit" like the other panes; helpBookmarks
  drops its own "q quit" so footer adds it consistently (no duplication).

The public sortEntries/filtered*/displayed*/cleanupTargets signatures are
unchanged, so the existing tests exercise the new shared code paths.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rjayasin @claude