User description
I'd like to share my theme that I've been using for a long time. Maybe you'll like it.
PR Type
Enhancement
Description
-
Add new "pino" theme with multi-line prompt layout
-
Display user, host, path on first line with icons
-
Support optional distrobox and VCS information display
-
Document new theme in THEMES.md with preview image
Diagram Walkthrough
flowchart LR
A["New pino Theme"] --> B["Multi-line Prompt"]
A --> C["Distrobox Support"]
A --> D["VCS Integration"]
B --> E["User@Host:Path"]
B --> F["Python venv Detection"]
C --> G["Container Info Display"]
D --> H["Git Status Indicators"]
A --> I["THEMES.md Documentation"]
File Walkthrough
| Relevant files |
|---|
| Enhancement |
pino.theme.shNew pino theme with multi-line prompt layout
themes/pino/pino.theme.sh
- New theme file implementing multi-line bash prompt with purple
box-drawing characters - Displays user, host, path on first line with optional venv indicator
- Includes distrobox container detection and OS information display
- Integrates VCS/Git status on separate line with clean/dirty indicators
- Uses color-coded segments (navy user, olive host, green path, teal
distro)
|
+117/-0 |
|
| Documentation |
THEMES.mdDocument pino theme in THEMES.md
themes/THEMES.md
- Add pino theme entry to documentation
- Include preview image reference for pino-dark.png
- Maintain alphabetical ordering in themes list
|
+4/-0 |
|
PR Compliance Guide 🔍
Below is a summary of compliance checks for this PR:
| Security Compliance |
| ⚪ |
Unsafe file sourcing
Description: Sourcing '/etc/os-release' without validation may execute shell code if the file is compromised; consider parsing with a safe method instead of 'source'. pino.theme.sh [48-55]
Referred Code
local NAME VERSION_ID
. /etc/os-release
local name="${NAME:-Linux}"
local ver="${VERSION_ID:-}"
name="${name%% *}"
|
| Ticket Compliance |
| ⚪ | 🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->
</details></td></tr>
| Codebase Duplication Compliance |
| ⚪ | Codebase context is not defined
Follow the guide to enable codebase context checks.
|
| Custom Compliance |
| ⚪ | No custom compliance provided
Follow the guide to enable custom compliance check.
|
- [ ] Update <!-- /compliance --update_compliance=true -->
| |
Compliance status legend
🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label
PR Code Suggestions ✨
Explore these optional code suggestions:
| Category | Suggestion | Impact |
| High-level |
Use core framework container detection utility
Replace the custom container detection logic in the theme with a standardized core utility from the oh-my-bash framework, likely found in lib/container.sh, to reduce code duplication.
Examples:
themes/pino/pino.theme.sh [22-42]
distrobox_instance() {
if [[ -n ${DISTROBOX_NAME:-} ]]; then
printf '%s' "$DISTROBOX_NAME"
return
fi
if [[ -n ${DBX_CONTAINER_NAME:-} ]]; then
printf '%s' "$DBX_CONTAINER_NAME"
return
fi
... (clipped 11 lines)
Solution Walkthrough:
Before:
distrobox_instance() {
if [[ -n ${DISTROBOX_NAME:-} ]]; then
printf '%s' "$DISTROBOX_NAME"
return
fi
if [[ -n ${DBX_CONTAINER_NAME:-} ]]; then
printf '%s' "$DBX_CONTAINER_NAME"
return
fi
if [[ -r /run/.containerenv ]]; then
# ... parse file
fi
if [[ -f /.dockerenv ]]; then
hostname
return
fi
return 1
}
_omb_theme_PROMPT_COMMAND() {
local dbx_name=$(distrobox_instance 2>/dev/null)
# ... use dbx_name
}
After:
# The custom `distrobox_instance` function is removed.
# A core utility like `_omb_container_name` from `lib/container.sh` should be used instead.
_omb_theme_PROMPT_COMMAND() {
# Assuming a core utility function is available, e.g., _omb_container_name
# This might require sourcing 'lib/container.sh' if not already done.
local dbx_name=$(_omb_container_name 2>/dev/null)
if [[ -n $dbx_name ]]; then
# ... build distrobox line
fi
# ...
}
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies that the theme reinvents container detection logic instead of using a likely-existing core utility, which is a significant maintainability and code duplication issue.
| Medium
|
| General |
✅ Explicitly handle function failure exit codes
Suggestion Impact:The commit changed the code to capture dbx_name via a command substitution within an if statement, checking success and defaulting to an empty string on failure, thus explicitly handling the function's exit code.
code diff:
- local dbx_name=$(distrobox_instance 2>/dev/null)
+ local dbx_name=""
+ if dbx_name=$(distrobox_instance 2>/dev/null); then
+ :
+ else
+ dbx_name=""
+ fi
+
Explicitly check the exit code of the distrobox_instance function to make failure handling more robust and the code's intent clearer.
themes/pino/pino.theme.sh [94]
-local dbx_name=$(distrobox_instance 2>/dev/null)
+local dbx_name
+if ! dbx_name=$(distrobox_instance 2>/dev/null); then
+ dbx_name=""
+fi
[Suggestion processed]
Suggestion importance[1-10]: 5
__
Why: The suggestion correctly points out that explicitly checking the function's exit code is more robust and improves code clarity, even though the original code functions as intended.
| Low
|
- [ ] Update <!-- /improve_multi --more_suggestions=true -->
| |
We'd probably merge the code for the container information with #683.