Skip to content

Use tmux to list keybindings#6308

Merged
dhh merged 3 commits into
basecamp:quattrofrom
heymaikol:simplify-tmux-keybindings
Jul 19, 2026
Merged

Use tmux to list keybindings#6308
dhh merged 3 commits into
basecamp:quattrofrom
heymaikol:simplify-tmux-keybindings

Conversation

@heymaikol

@heymaikol heymaikol commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the hand-rolled parser with annotated tmux list-keys -N output
  • keep the 42 shipped bindings curated by storing descriptions beside each bind
  • make --config authoritative on a per-invocation isolated socket that is always cleaned up

This removes 234 lines net without touching the default tmux server.

Tests

  • bash -n bin/omarchy-menu-tmux-keybindings
  • verified exactly 42 annotated bindings and no built-in defaults
  • verified an alternate config wins while a default server is running
  • verified exit-empty off leaves no server or socket behind
  • verified the default server prefix and bindings remain unchanged
  • ./test/all

Copilot AI review requested due to automatic review settings July 19, 2026 01:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dhh dhh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the instinct here — the awk parser deserved to die, and letting tmux report its own state is the right direction. But as written this trades away more than it deletes. Three things to fix before this can go in:

1. Output readability. The old viewer printed ~45 curated lines like PREFIX + c → New window. This prints 310 raw bind-key lines, ~265 of which are tmux's built-in copy-mode defaults, drowning out the Omarchy bindings — and it's feeding end-user surfaces (the menu popup and PREFIX + ?). Suggestion: use tmux list-keys -N instead, and add -N "description" notes to the binds in config/tmux/tmux.conf. That gives clean, annotated output with the prefix already resolved (C-Space c Create a new window), moves the descriptions into the config next to the bindings they describe, and still deletes the whole parser.

2. --config is silently ignored when a server is running. tmux only reads -f at server start, so with a live server (which is always the case for the PREFIX + ? binding) the flag does nothing — verified with a running server on one config and -f pointing at another. Pick one semantic: either config-file-authoritative (run on an isolated socket, e.g. tmux -L omarchy-keybindings -f "$config_file") or live-server (drop --config and the file checks). Right now the answer depends on invisible state.

3. Side effects on the default socket. When no server is running, this starts a real one and fully sources the user's config (including any run-shell hooks) just to print keybindings — and a config with set -g exit-empty off leaves that server orphaned. An isolated -L socket for the cold path fixes this too.

Also a nit: the omarchy:summary comment still says "defined in your configuration," which stops being true once defaults are listed.

Copilot AI review requested due to automatic review settings July 19, 2026 01:47
@heymaikol

heymaikol commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

@dhh Addressed in b2b9ed6:

  1. Added -N notes beside all 42 shipped bindings and switched the viewer to curated list-keys -N output.
  2. Made --config authoritative by sourcing it on a unique isolated socket.
  3. Added guaranteed server/socket cleanup, including configs with exit-empty off.
  4. Updated the stale summary.

Verified the output count/readability, alternate-config behavior with a live default server, unchanged default-server state, socket cleanup, and ./test/all.

EDIT: Used at dhh because I changed commit author to use my username

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated no new comments.

@heymaikol
heymaikol force-pushed the simplify-tmux-keybindings branch from b2b9ed6 to 051f89b Compare July 19, 2026 01:52
… own

list-keys -aN falls back to the raw command when a binding has no note,
so user-added bindings in an existing tmux.conf still show up. Dropping
exit-empty off means the throwaway server exits when the client
disconnects instead of relying solely on the trap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 19, 2026 16:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@dhh
dhh merged commit 3754c2c into basecamp:quattro Jul 19, 2026
katrushenkov pushed a commit to katrushenkov/omarchy that referenced this pull request Jul 20, 2026
* Use tmux to list keybindings

* Annotate tmux keybindings

* List unannotated bindings too and let the isolated server exit on its own

list-keys -aN falls back to the raw command when a binding has no note,
so user-added bindings in an existing tmux.conf still show up. Dropping
exit-empty off means the throwaway server exits when the client
disconnects instead of relying solely on the trap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants