Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts line-wrapping behavior for app descriptions and app name listings by switching from simple width-based wrapping to fmt’s goal width option so wrapped text more closely matches the available column width in menu displays. Flow diagram for env_format_lines app description wrappingflowchart TD
A["Get APPNAME"] --> B["Call run_script app_nicename APPNAME"]
B --> C["Set AppName from nicename"]
A --> D["Call run_script app_description APPNAME"]
D --> E["Pipe description to fmt -w 75 -g 75"]
E --> F["Set AppDescription to wrapped text"]
F --> G["Build HeadingTitle from AppName and tags"]
G --> H["Use HeadingTitle and AppDescription in heading output"]
Flow diagram for menu_app_select init_gauge_text name column wrappingflowchart TD
A["Prepare AppNamesArray"] --> B["printf each app name on its own line"]
B --> C["Pipe to fmt -w (TextCols - AppsColumnStart) -g (TextCols - AppsColumnStart)"]
C --> D["Pipe to pr -e -t -o AppsColumnStart for column alignment"]
D --> E["Store result in FormattedAppNames"]
E --> F["Display FormattedAppNames in gauge text"]
Flow diagram for menu_heading app description wrapping with goal widthflowchart TD
A["Get ScreenCols from stty size"] --> B["Compute TextWidth = ScreenCols - WindowColsAdjust - TextColsAdjust - LabelWidth"]
B --> C["Call fmt -w TextWidth -g TextWidth with AppDescription via here string"]
C --> D["readarray AppDesciptionArray from fmt output"]
D --> E["Format each element with Indent and HeadingAppDescription color"]
E --> F["Append formatted lines and blank line to Heading Application entry"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The switch from
fold -stofmtmay change how existing multi-line descriptions are wrapped (e.g., merging intentional line breaks into single paragraphs); please confirm this behavior is acceptable for all currentapp_descriptionoutputs or consider constrainingfmtusage to avoid altering paragraph structure where that matters. - In
menu_heading.sh, the here-string invocationfmt -w ${TextWidth} -g ${TextWidth}<<< "${AppDescription}"is missing a space before<<<, which hurts readability and may trip shell tooling; update to... -g ${TextWidth} <<< "${AppDescription}"for consistency with surrounding code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The switch from `fold -s` to `fmt` may change how existing multi-line descriptions are wrapped (e.g., merging intentional line breaks into single paragraphs); please confirm this behavior is acceptable for all current `app_description` outputs or consider constraining `fmt` usage to avoid altering paragraph structure where that matters.
- In `menu_heading.sh`, the here-string invocation `fmt -w ${TextWidth} -g ${TextWidth}<<< "${AppDescription}"` is missing a space before `<<<`, which hurts readability and may trip shell tooling; update to `... -g ${TextWidth} <<< "${AppDescription}"` for consistency with surrounding code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
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.
Pull request
Purpose
Describe the problem or feature in addition to a link to the issues.
Approach
How does this change address the problem?
Open Questions and Pre-Merge TODOs
Check all boxes as they are completed
Learning
Describe the research stage
Links to blog posts, patterns, libraries or addons used to solve this problem
Requirements
Check all boxes as they are completed
Summary by Sourcery
Improve text wrapping behavior for application descriptions and selection menus to better respect the configured column width.
Bug Fixes:
Enhancements: