-
Notifications
You must be signed in to change notification settings - Fork 40
feat(abctl): make the [?] overlay a map of the panes, not a key dump #1100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
93a0a5a
945ccff
d558543
3718009
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -821,8 +821,15 @@ func (m *model) backToPodsPane() { | |
| // every resize while it's open, so the body re-wraps and the scroll range | ||
| // stays correct. resetScroll is true only on open — a resize should keep | ||
| // the reader where they were. | ||
| // | ||
| // THE BODY ACTUALLY RE-WRAPS NOW. This comment claimed it did while | ||
| // helpBodyLines took no width at all — it built one width-blind string and the | ||
| // viewport clipped whatever overran, so the overlay's longest line lost its | ||
| // second half on an 80-column terminal and said nothing about it. The wrap | ||
| // budget is the terminal minus the frame the panel draws around the viewport. | ||
| func (m *model) syncHelpViewport(resetScroll bool) { | ||
| body := helpBodyLines(m.pane) | ||
| frameW := styleBorder.GetHorizontalBorderSize() + helpPadX*2 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: A small |
||
| body := helpBodyLines(m.pane, m.width-frameW) | ||
| w, h := helpViewportSize(m.width, m.height, helpBodyWidth(body)) | ||
| m.helpVp.Width = w | ||
| m.helpVp.Height = h | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion — the enumeration claims to be exhaustive and is missing half its cases.
This bullet says the section is "rendered per pane and only where the key actually works", then lists three cases: all four from the session views,
Calone on Usage, and the two pickers. Because the sentence's whole point is that the set is pane-specific, a reader takes the list as complete — so the three unlisted panes read as having no jump keys at all.I dumped
jumpsFromacross every pane on this branch:u P C $(as documented)C $C $$C(as documented)Six distinct cases, three documented. CodeRabbit flagged this independently and is right; its suggested wording covers it.
Worth noting where this lands, because it is the PR's own thesis: the old overlay advertised keys that did nothing on panes it never checked, and the fix was to derive availability per pane instead of writing prose beside a key (
jumpsFrom, and "(not on usage)" giving way tospendDrawerHostPane). The README is now the one copy of that mapping still maintained by hand — the same shape of defect, one layer out. Nothing is broken for users: the overlay is correct andTestHelpBody_JumpSectionMatchesTheKeysThatActuallyWorkholds it there. Only the prose can drift, and it has.