oh-my-bash icon indicating copy to clipboard operation
oh-my-bash copied to clipboard

add new theme pino

Open Tealk opened this issue 3 months ago • 3 comments

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.sh
New 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.md
Document 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     

Tealk avatar Oct 16 '25 20:10 Tealk

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:

CategorySuggestion                                                                                                                                    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.

akinomyoga avatar Oct 17 '25 03:10 akinomyoga