Size the whiptail dialogs to the terminal and their contents - #11
Merged
Conversation
The container picker was hard-coded to `20 $width 10`: 20 rows tall, list height 10. Three problems came out of that, all visible in one screenshot. Three dead rows under the list. A checklist needs exactly list-height + 8 rows -- top border, the prompt's blank/text/blank, a blank, the button row, a blank, bottom border -- and 20 against a list height of 10 is two more than that. I measured it rather than guessing: at +7 whiptail silently drops the prompt text instead of shrinking the box, and at +8 it fits exactly. A ten-item window regardless of the screen. On a 45-row terminal the dialog stayed 20 rows and showed 10 of 40 containers. The list height is now the container count, capped by what the terminal can hold; the same fleet on the same terminal now shows 35. A width that ignored half its inputs. It was the longest hostname plus 12, which left out the VMID column entirely -- so a node whose IDs have run past four digits had those rows clipped. Width is now the widest of a list row (measured id and name widths), the title, and the prompt. When the list does overflow, the prompt says how many there are in total. whiptail draws its own scrollbar, so the arrows are not the missing information; the count is, because "8 of 40" is a different decision from "8 of 9". That notice caught its own bug in testing: at 40 columns it rendered as "Select containers to skip (10 total," -- clipped mid-sentence, worse than saying nothing -- which is why the prompt is now part of the width calculation. confirm() gets the same treatment. It was fixed at 60 columns, so a node with a long fully-qualified hostname had the question wrapped awkwardly against the frame; it is now sized to the message and bounded by the terminal. term_size falls back to 24x80 when tput cannot answer, which covers a missing TERM and a non-tty. select_exclusions only runs when main has already established a terminal, so the fallback is belt and braces. Verified by rendering the real function in a pty at 14, 18 and 45 rows and reading the frames back: no dead rows, prompt intact, scrollbar and total shown only when the list actually overflows, long names and five-digit VMIDs uncut, and a 43-character FQDN wrapping inside the frame rather than through it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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
1 participant
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 container picker in
update-lxcs.shwas hard-coded to20 $width 10. Three problems came out of that, all visible in one screenshot.Three dead rows under the list. A checklist needs exactly list-height + 8 rows. I measured rather than guessed: at +7 whiptail silently drops the prompt text instead of shrinking the box; at +8 it fits exactly. The old 20-against-10 was two too many.
A ten-item window regardless of screen size. On a 45-row terminal the dialog stayed 20 rows and showed 10 of 40 containers. List height is now the container count capped by the terminal — the same fleet on the same terminal now shows 35.
A width that ignored half its inputs. It was longest-hostname + 12, leaving out the VMID column entirely, so a node whose IDs have run past four digits had those rows clipped. Width is now the widest of a list row (measured id and name widths), the title, and the prompt.
When the list overflows, the prompt now gives the total. whiptail draws its own scrollbar, so the arrows aren't the missing information — the count is, because "8 of 40" is a different decision from "8 of 9".
That notice caught its own bug during testing: at 40 columns it rendered as
Select containers to skip (10 total,— clipped mid-sentence, worse than saying nothing. That's why the prompt is now part of the width calculation.confirm()gets the same treatment; it was fixed at 60 columns, so a long FQDN wrapped awkwardly against the frame.Verification
Rendered the real function in a pty at 14, 18 and 45 rows and read the frames back — no dead rows, prompt intact, scrollbar and total shown only when the list actually overflows, long names and five-digit VMIDs uncut, and a 43-character FQDN wrapping inside the frame rather than through it.
term_sizefalls back to 24x80 whentputcannot answer (missing TERM, no tty), verified both.