-
Notifications
You must be signed in to change notification settings - Fork 1
migrate #23
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
new file: interface/info_box.sh new file: interface/ok_box.conf new file: interface/ok_box.sh new file: interface/test_info_box.sh new file: interface/test_ok_box.sh
renamed: interface/info_box.sh -> staging/interface/info_box.sh renamed: interface/ok_box.conf -> staging/interface/ok_box.conf renamed: interface/ok_box.sh -> staging/interface/ok_box.sh renamed: interface/test_info_box.sh -> staging/interface/test_info_box.sh renamed: interface/test_ok_box.sh -> staging/interface/test_ok_box.sh
WalkthroughNew interface modules for displaying informational and OK message boxes in terminal user interfaces have been introduced, including their configuration files, implementation scripts, and placeholder test scripts. The modules support both dynamic and static message display using dialog or whiptail, with help and usage documentation provided in corresponding sections. A minor typo fix was made in the debug configuration. Additionally, the previously existing templates module files and configuration were removed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Terminal
participant info_box.sh
participant DialogTool
User->>Terminal: Run info_box (with pipe or argument)
Terminal->>info_box.sh: Execute script
info_box.sh->>DialogTool: Show info box (dynamic or static message)
DialogTool-->>User: Display info box
sequenceDiagram
participant User
participant Terminal
participant ok_box.sh
participant DialogTool
User->>Terminal: Run ok_box (with argument or stdin)
Terminal->>ok_box.sh: Execute script
ok_box.sh->>DialogTool: Show OK message box
DialogTool-->>User: Display message box
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 6
♻️ Duplicate comments (1)
staging/interface/test_ok_box.sh (1)
6-10: Same CI-breaking exit code as intest_info_box.shAdjust as above to avoid false-negative pipeline runs while the test body is still a stub.
🧹 Nitpick comments (6)
staging/interface/info_box.sh (2)
1-2: Add strict-mode guards (set -euo pipefail) at the top of the scriptEnabling
set -euo pipefailmakes the helper fail fast on errors, unset variables, or broken pipelines – behaviour already used in your test stubs and most Armbian-config helpers.
It prevents silent UI freezes if, for example,$dialogis missing orprintffails.#!/usr/bin/env bash +set -euo pipefail
42-44: Hard-coded 0.5 s refresh may be too slow/fast for some use-casesConsider making the sleep interval configurable via an environment variable (e.g.
INFOBOX_REFRESH_MS) or cli flag so callers can tune UI responsiveness vs. CPU usage.staging/interface/ok_box.sh (4)
18-21: Extend help flags to include--help.Currently only
helpand-hinvoke_about_interface_message. For consistency, accept--help(and optionally-?):-if [[ "$message" == "help" || "$message" == "-h" ]]; then +if [[ "$message" =~ ^(-h|--help|\?)$ ]]; then _about_interface_message return 0 fi
29-30: Verify that the chosen dialog tool is installed.Before the
casestatement, ensure$dialogexists on PATH to avoid cryptic failures:+if ! command -v "$dialog" &>/dev/null; then + echo "Error: '$dialog' is not installed" >&2 + return 1 +fi local dialog="${DIALOG:-whiptail}"
31-37: Handle multiline and special‐character messages safely.Passing unescaped quotes or newlines directly to
--msgboxcan break the dialog. Consider writing the message to a temporary file or usingprintf:+printf '%s\n' "$message" >"$tmpfile" +dialog --title "${TITLE:-Info}" --msgbox "$tmpfile" 10 80 >/dev/tty 2>&1
41-44: Clarify unsupported$DIALOGbehavior.Falling back to
echo "$message"with exit code1is ambiguous. Better to emit an explicit error, show usage, and then exit:*) - echo "$message" - return 1 + echo "Error: Unsupported dialog type '$dialog'" >&2 + _about_interface_message + return 1 ;;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
staging/interface/info_box.conf(1 hunks)staging/interface/info_box.sh(1 hunks)staging/interface/ok_box.conf(1 hunks)staging/interface/ok_box.sh(1 hunks)staging/interface/test_info_box.sh(1 hunks)staging/interface/test_ok_box.sh(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: 01 Coding style check
staging/interface/info_box.conf
[error] 1-1: editorconfig-checker: Wrong line endings or no final newline. 1 error found.
🔇 Additional comments (1)
staging/interface/info_box.conf (1)
3-5: Mixed attribute names:extend_descvsdocumentsvsextend_docsFor consistency with other config files and to satisfy sanity-checkers, prefer:
extend_desc=... extend_docs=false
documents=falseis non-standard and will likely be ignored.
modified: staging/interface/info_box.sh modified: staging/interface/ok_box.conf modified: staging/interface/ok_box.sh
modified: staging/interface/ok_box.sh
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.
Actionable comments posted: 2
♻️ Duplicate comments (2)
staging/interface/ok_box.conf (1)
4-4:extend_disctypo still present – please propagate the earlier fixThe key is still spelled
extend_disc; all other.conffiles (and the parser) expectextend_desc.
This was flagged in the previous review and remains unresolved.-extend_disc= +extend_desc=Also applies to: 19-19
staging/interface/ok_box.sh (1)
4-7: Usage line still mismatched – replaceinterface_messagewithok_boxThe Usage header should match the actual entry point.
-Usage: interface_message ["message"] +Usage: ok_box ["message"]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
staging/interface/info_box.conf(1 hunks)staging/interface/info_box.sh(1 hunks)staging/interface/ok_box.conf(1 hunks)staging/interface/ok_box.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- staging/interface/info_box.conf
- staging/interface/info_box.sh
modified: staging/interface/info_box.conf modified: staging/interface/ok_box.conf
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
deleted: templates/templates.conf deleted: templates/templates.sh deleted: templates/test_templates.sh
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.
Pull Request Overview
This PR migrates the codebase by removing unused template files and introducing new interactive terminal modules for displaying OK and info boxes. The changes include the deletion of templates-related scripts and configuration, the addition of new scripts for ok_box and info_box (along with their tests and configs), and a minor correction in the debug configuration.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/templates.sh | Removed unused bash module for templates |
| templates/templates.conf | Removed unused configuration file for templates |
| templates/doc_templates.md | Removed extra documentation file for templates |
| staging/interface/test_ok_box.sh | Added placeholder test script for ok_box module |
| staging/interface/test_info_box.sh | Updated test label from "templates" to "info_box" in the test script |
| staging/interface/ok_box.sh | Introduced interactive ok_box module with support for multiple dialogs |
| staging/interface/ok_box.conf | Added configuration for the ok_box module |
| staging/interface/info_box.sh | Introduced interactive info_box module with live updates |
| staging/interface/info_box.conf | Added configuration for the info_box module |
| src/initialize/debug.conf | Corrected a configuration key typo (extend_disc ➜ extend_desc) |
staging/interface/ok_box.sh
Outdated
| echo "$message" | ||
| ;; | ||
| *) | ||
| echo "$message" |
Copilot
AI
Jun 17, 2025
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 the default case of the switch (lines 48-50), consider outputting a clear error message indicating that an unsupported dialog tool was provided, rather than just echoing the message.
| echo "$message" | |
| echo "Error: Unsupported dialog tool '$dialog'. Supported tools are: dialog, whiptail, read." >&2 |
Co-authored-by: Copilot <[email protected]>
deleted: staging/interface/info_box.sh deleted: staging/interface/ok_box.conf deleted: staging/interface/ok_box.sh deleted: staging/interface/test_info_box.sh deleted: staging/interface/test_ok_box.sh
Summary by CodeRabbit