feat: add Bold Brew desktop entry #33
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds desktop integration for the Bold Brew application, a terminal UI for managing Homebrew packages. The changes enable users to launch Bold Brew from their desktop environment's application menu.
Key Changes:
- Added a desktop entry file (
bbrew.desktop) following the freedesktop.org specification - Created a launcher script (
bbrew-launch) that handles Homebrew environment setup and auto-installation - Included an application icon (
bbrew.png) for display in application menus
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
system_files/shared/usr/share/icons/hicolor/256x256/apps/bbrew.png |
Application icon in PNG format (256x256 resolution) |
system_files/shared/usr/share/applications/bbrew.desktop |
Desktop entry file defining how the application appears in menus |
system_files/shared/usr/libexec/bbrew-launch |
Bash launcher script that sets up Homebrew environment and handles first-time installation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [[ -x "$BBREW_PATH" ]]; then | ||
| exec "$BBREW_PATH" "$@" | ||
| else | ||
| echo "Bold Brew first time setup. Installing..." |
There was a problem hiding this comment.
The informational message should be directed to stderr instead of stdout since this is a status message and not data output. This prevents the message from interfering with any command output. Change to: echo "Bold Brew first time setup. Installing..." >&2
| echo "Bold Brew first time setup. Installing..." | |
| echo "Bold Brew first time setup. Installing..." >&2 |
| eval "$($BREW_PATH shellenv)" | ||
|
|
There was a problem hiding this comment.
The eval "$($BREW_PATH shellenv)" command could be a security risk if BREW_PATH or its output is compromised. Consider adding validation to verify BREW_PATH hasn't been tampered with, or at minimum add a comment documenting this security consideration. Additionally, the script should handle the case where shellenv fails or returns unexpected output.
| eval "$($BREW_PATH shellenv)" | |
| # SECURITY NOTE: The following command executes shell code generated by Homebrew. | |
| # If $BREW_PATH or its output is compromised, this could be a security risk. | |
| # We validate $BREW_PATH and check shellenv output before eval. | |
| # Validate that BREW_PATH is the expected binary (optional: check hash or symlink) | |
| if ! [[ "$(readlink -f "$BREW_PATH")" == "/home/linuxbrew/.linuxbrew/bin/brew" ]]; then | |
| echo "FATAL: $BREW_PATH does not resolve to expected Homebrew binary." | |
| exit 1 | |
| fi | |
| SHELLENV_OUTPUT="$($BREW_PATH shellenv 2>/dev/null)" | |
| if [[ $? -ne 0 || -z "$SHELLENV_OUTPUT" ]]; then | |
| echo "FATAL: Failed to get shellenv from Homebrew." | |
| exit 1 | |
| fi | |
| # Basic validation: ensure output only contains variable assignments and 'export' | |
| if ! echo "$SHELLENV_OUTPUT" | grep -Eq '^(export [A-Z_]+=|PATH=|MANPATH=|INFOPATH=)'; then | |
| echo "FATAL: Unexpected output from brew shellenv." | |
| exit 1 | |
| fi | |
| eval "$SHELLENV_OUTPUT" |
| exec "$BBREW_PATH" "$@" | ||
| else | ||
| echo "Bold Brew first time setup. Installing..." | ||
| "$BREW_PATH" install Valkyrie00/homebrew-bbrew/bbrew |
There was a problem hiding this comment.
The brew install command runs without any confirmation or error handling beyond the final existence check. If the installation fails silently or is interrupted, the user won't get helpful feedback. Consider capturing the installation output and providing more specific error messages, such as: if ! "$BREW_PATH" install Valkyrie00/homebrew-bbrew/bbrew; then echo "FATAL: Installation command failed. Check brew logs for details." >&2; exit 1; fi
| "$BREW_PATH" install Valkyrie00/homebrew-bbrew/bbrew | |
| if ! "$BREW_PATH" install Valkyrie00/homebrew-bbrew/bbrew; then | |
| echo "FATAL: Installation command failed. Check brew logs for details." >&2 | |
| exit 1 | |
| fi |
| Terminal=true | ||
| Type=Application | ||
| Categories=Development;System; | ||
| Keywords=homebrew;brew;package |
There was a problem hiding this comment.
The Keywords field is missing a semicolon at the end. According to the Desktop Entry Specification, list values should end with a semicolon. This should be Keywords=homebrew;brew;package; to properly conform to the specification.
| Keywords=homebrew;brew;package | |
| Keywords=homebrew;brew;package; |
| Icon=bbrew | ||
| Terminal=true | ||
| Type=Application | ||
| Categories=Development;System; |
There was a problem hiding this comment.
The Categories field is missing a semicolon at the end. According to the Desktop Entry Specification, list values should end with a semicolon. This should be Categories=Development;System; to properly conform to the specification.
| BREW_PATH="/home/linuxbrew/.linuxbrew/bin/brew" | ||
|
|
||
| if [[ ! -x "$BREW_PATH" ]]; then | ||
| echo "FATAL: Homebrew is not installed at $BREW_PATH" |
There was a problem hiding this comment.
The error message should be directed to stderr instead of stdout. Error messages should use >&2 to ensure they appear in the error stream. Change to: echo "FATAL: Homebrew is not installed at $BREW_PATH" >&2
| echo "FATAL: Homebrew is not installed at $BREW_PATH" | |
| echo "FATAL: Homebrew is not installed at $BREW_PATH" >&2 |
| if [[ -x "$BBREW_PATH" ]]; then | ||
| exec "$BBREW_PATH" "$@" | ||
| else | ||
| echo "FATAL: Installation failed." |
There was a problem hiding this comment.
The error message should be directed to stderr instead of stdout. Error messages should use >&2 to ensure they appear in the error stream. Change to: echo "FATAL: Installation failed." >&2
| echo "FATAL: Installation failed." | |
| echo "FATAL: Installation failed." >&2 |
1 participant
No description provided.