Skip to content

Conversation

@Tealk
Copy link

@Tealk Tealk commented Oct 16, 2025

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"]
Loading

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     

@qodo-free-for-open-source-projects
Copy link

qodo-free-for-open-source-projects bot commented Oct 16, 2025

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 status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-free-for-open-source-projects
Copy link

qodo-free-for-open-source-projects bot commented Oct 16, 2025

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

@akinomyoga
Copy link
Contributor

We'd probably merge the code for the container information with #683.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants