-
Notifications
You must be signed in to change notification settings - Fork 9
Improve automatic TUI sizing algorithm #26
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
Conversation
Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>
Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>
Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>
lunarcloud
left a comment
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.
In testing, the size looks too wide.
Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>
Fixed in commit 0bbf797. Short messages (≤40 chars) now use compact sizing (40 cols) instead of expanding to 69 cols. Longer messages (>40 chars) still expand appropriately for readability. Before: After: |
lunarcloud
left a comment
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.
Checklist and Radiolist dialogs still seemed too wide in TUIs, can we fix that?
Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>
Fixed in commit 0d274e1. Checklist and radiolist were using MAX_COLS (116) instead of RECMD_COLS, making them unnecessarily wide. Now they use adaptive width based on prompt text length. Before (116 cols): After (62 cols): Width reduced by 54 columns for this prompt, and by 76 columns for shorter prompts like "Favorite Primary Color?" |
lunarcloud
left a comment
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.
checklist and radiolist are not tall enough, I have to scroll for the small number of items in the test.sh file - try to fix this.
Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>
Fixed in commit d637dc4. The height calculation was based only on the prompt text, not the number of options. Now both checklist and radiolist adjust their height based on Before:
After:
The test.sh examples with 4 options now display without any scrolling. |
lunarcloud
left a comment
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.
The height for the "whiptail" interface is tall enough now, but the "dialog" interface needs to be a little bit taller. It seems the difference is that whiptail doesn't show the message but dialog doesn't.
Co-authored-by: lunarcloud <1565970+lunarcloud@users.noreply.github.com>
Fixed in commit 69384da. You're correct - whiptail displays the message text in the title bar, but dialog displays it in the dialog body. The height calculation now adds extra lines for the message text when using the dialog interface. Whiptail (text in title - 10 lines): Dialog (text in body - 11 lines, +1 for message): Both interfaces now display properly without requiring scrolling for the test.sh examples. |
Improvements to automatic TUI sizing:
Changes
Rewrote
_calculate-tui-size()function:chars/width, properly handling newlines(MAX_COLS - MIN_COLS) * 3/4text_width ≤ 0Fixed single-line text width calculation:
Fixed checklist and radiolist width:
Fixed checklist and radiolist height:
NUM_OPTIONS + 6(for UI elements) to show all items without scrollingFixed dialog interface height for checklist and radiolist:
Results
Before fixes:
After fixes:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.