Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new devcontainer feature for installing and configuring Oh My Posh, a cross-shell prompt theme engine. The implementation includes installation scripts, test scenarios, and comprehensive documentation.
Key changes:
- New Oh My Posh feature with configurable version, theme, install path, and shell support (bash, zsh, fish)
- Installation script with architecture detection, binary download from GitHub releases, and shell configuration
- Test suite covering default installation, custom versions, themes, install paths, and shell-specific configurations
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ohmyposh/install.sh | Core installation script handling binary download, installation, and shell configuration |
| src/ohmyposh/devcontainer-feature.json | Feature metadata and configuration options schema |
| test/ohmyposh/*.sh | Test scripts for various feature scenarios |
| test/ohmyposh/scenarios.json | Test scenario configurations |
| .devcontainer/ohmyposh/* | Local testing copies of feature files |
| README.md | Updated documentation with Oh My Posh usage examples |
| docs/ohmyposh-plan.md | Implementation plan and design decisions |
| justfile | Build automation for feature development |
| .github/workflows/test.yaml | Updated CI workflow to test ohmyposh feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ -s ~/.ohmyposh.json ]; then | ||
| # Use custom theme if mounted | ||
| eval "\$(oh-my-posh init $SHELL_CMD --config ~/.ohmyposh.json)" | ||
| else | ||
| # Use built-in theme | ||
| eval "\$(oh-my-posh init $SHELL_CMD --config $THEME)" | ||
| fi |
There was a problem hiding this comment.
The shell configuration uses bash syntax [ -s ... ] which is incompatible with fish shell. When this code is appended to fish's config.fish file, it will cause syntax errors. Fish uses different conditional syntax: test -s or [ -s ] within if statements, but with different syntax structure (e.g., if test -s ~/.ohmyposh.json followed by code, then else, then end).
| "terminal.integrated.fontFamily": "Hack Nerd Font, MesloLGS NF, FiraCode Nerd Font, JetBrainsMono Nerd Font, monospace", | ||
| "terminal.integrated.fontLigatures.enabled": true |
There was a problem hiding this comment.
The .devcontainer/ohmyposh/devcontainer-feature.json file includes a setting terminal.integrated.fontLigatures.enabled that is not present in the source file src/ohmyposh/devcontainer-feature.json. These files should be in sync since the justfile copies from src to .devcontainer. This inconsistency will cause confusion and potential issues.
| "terminal.integrated.fontFamily": "Hack Nerd Font, MesloLGS NF, FiraCode Nerd Font, JetBrainsMono Nerd Font, monospace", | |
| "terminal.integrated.fontLigatures.enabled": true | |
| "terminal.integrated.fontFamily": "Hack Nerd Font, MesloLGS NF, FiraCode Nerd Font, JetBrainsMono Nerd Font, monospace" |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| check "oh-my-posh is installed" oh-my-posh version | ||
|
|
||
| # Only bash should be configured when shells="bash" | ||
| check "bash is configured" grep -q "oh-my-posh init bash" ~/.bashrc || echo "Warning: bash not configured" |
There was a problem hiding this comment.
The 'echo' statement prevents test failure when grep doesn't find a match. The '|| echo' makes the command always succeed (exit 0), which means the test passes even if bash isn't configured. Remove '|| echo "Warning: bash not configured"' to properly fail the test when bash configuration is missing.
| check "bash is configured" grep -q "oh-my-posh init bash" ~/.bashrc || echo "Warning: bash not configured" | |
| check "bash is configured" grep -q "oh-my-posh init bash" ~/.bashrc |
This pull request introduces a new "Oh My Posh" devcontainer feature, providing a robust mechanism to install and configure the Oh My Posh shell prompt engine in development containers. The implementation includes configuration options, installation scripts, documentation, and workflow/test updates to support and validate the feature.
The most important changes are:
Oh My Posh Feature Implementation:
ohmyposhfeature with a comprehensivedevcontainer-feature.jsondefining options for version, theme, install path, and supported shells. This enables flexible and user-friendly configuration for different environments. [1] [2]install.shscript that downloads the correct Oh My Posh binary for the container architecture, installs it, sets up a placeholder theme file, and configures the appropriate shell initialization scripts for bash, zsh, and fish. The script includes logic to use a custom theme if mounted, or fall back to a built-in theme.Documentation and Planning:
README.mdto document the new Oh My Posh feature, including usage examples, available options, instructions for mounting custom themes, and publishing guidance.docs/ohmyposh-plan.md, outlining design decisions, configuration, installation logic, and testing scenarios for the feature.Development and Testing Enhancements:
justfilewith commands to build, copy, and clean the Oh My Posh feature for local development and testing..devcontainer/devcontainer.json) to include the new Oh My Posh feature for local testing..github/workflows/test.yaml) to test the Oh My Posh feature on multiple base images, ensuring compatibility and reliability. [1] [2]