feat: support OmarchyBar and Waybar integrations - #6
Merged
Conversation
📝 WalkthroughWalkthroughThe Linux installation flow now uses a shared launcher and configurable QuickDrop backend. It detects and installs OmarchyBar, Waybar, both, or neither. Waybar patching moved to Python, while OmarchyBar receives a packaged widget and manifest. ChangesLinux bar integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Desktop as install-desktop.ts
participant Linux as install-linux.sh
participant Integration as install-bar-integration.sh
participant Waybar
participant Omarchy as OmarchyBar
Desktop->>Linux: start Linux installation
Linux->>Linux: install launcher and integration assets
Linux->>Integration: select bar integration
Integration->>Waybar: patch JSONC module when selected
Integration->>Omarchy: install and enable widget when selected
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
scripts/install-waybar-module.test.ts (1)
19-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for a config without
modules-right.The current cases always contain
modules-right, soensure_in_modules_rightnever takes its early-return branch. That branch is the one that produces no visible module. A test pins the intended behavior after the warning is added.💚 Proposed test
+ test("keeps the config usable when modules-right is absent", async () => { + const { text } = await patch(`{ + "layer": "top", + "tray": { "icon-size": 12 } +}`); + + expect(text).toContain(`"custom/quickdrop": {`); + expect(text).not.toContain(`"modules-right"`); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-waybar-module.test.ts` around lines 19 - 31, Add a test case alongside the existing QuickDrop installation test that passes configuration without a modules-right entry, exercising ensure_in_modules_right’s early-return path and verifying the resulting configuration contains no visible QuickDrop module.scripts/install-waybar-module.py (1)
150-157: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: report a missing config path with a clear message.
If the path does not exist,
read_textraisesFileNotFoundErrorand prints a traceback. The caller inscripts/install-bar-integration.shthen shows that traceback to the user. ASystemExitmessage matches the usage error on line 152.♻️ Proposed refactor
config_path = Path(sys.argv[1]) launcher_path = sys.argv[2] + if not config_path.is_file(): + raise SystemExit(f"config not found: {config_path}") original = config_path.read_text(encoding="utf-8")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-waybar-module.py` around lines 150 - 157, Update main around config_path.read_text to catch FileNotFoundError for the configured path and raise SystemExit with a clear missing-config message, avoiding the traceback while preserving the existing usage validation and patch flow.src/server/index.ts (1)
60-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: register the Linux asset routes from a table.
The six registrations differ only by route and file name.
scripts/install-linux.shalready derives exactly these paths, so keeping them in one list reduces drift when an asset is added.♻️ Proposed refactor
- app.get("/linux/quickdrop-launcher", async (_request, reply) => - sendScriptFile(reply, "quickdrop-launcher"), - ); - // Backward-compatible endpoint for existing Waybar-only installers. - app.get("/linux/quickdrop-waybar", async (_request, reply) => - sendScriptFile(reply, "quickdrop-launcher"), - ); - app.get("/linux/install-bar-integration", async (_request, reply) => - sendScriptFile(reply, "install-bar-integration.sh"), - ); - app.get("/linux/install-waybar-module.py", async (_request, reply) => - sendScriptFile(reply, "install-waybar-module.py"), - ); - app.get("/linux/omarchy/manifest.json", async (_request, reply) => - sendScriptFile(reply, "omarchy-quickdrop/manifest.json"), - ); - app.get("/linux/omarchy/BarWidget.qml", async (_request, reply) => - sendScriptFile(reply, "omarchy-quickdrop/BarWidget.qml"), - ); + const linuxAssets: Record<string, string> = { + "/linux/quickdrop-launcher": "quickdrop-launcher", + // Backward-compatible endpoint for existing Waybar-only installers. + "/linux/quickdrop-waybar": "quickdrop-launcher", + "/linux/install-bar-integration": "install-bar-integration.sh", + "/linux/install-waybar-module.py": "install-waybar-module.py", + "/linux/omarchy/manifest.json": "omarchy-quickdrop/manifest.json", + "/linux/omarchy/BarWidget.qml": "omarchy-quickdrop/BarWidget.qml", + }; + for (const [route, fileName] of Object.entries(linuxAssets)) { + app.get(route, async (_request, reply) => sendScriptFile(reply, fileName)); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/index.ts` around lines 60 - 78, Optionally consolidate the six Linux asset registrations into a single route-to-file table and iterate over it to call sendScriptFile. Preserve every existing route, including the backward-compatible quickdrop-waybar endpoint and all launcher, integration, Python, manifest, and QML asset mappings.scripts/install-bar-integration.sh (3)
168-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport a failed launcher persistence instead of hiding it.
|| truediscards the failure. Ifomarchy bar setfails, the widget keeps its default launcher path, and a customQUICKDROP_BIN_DIRinstall then starts nothing. Emit a warning so the user can run the command manually.♻️ Proposed change
- omarchy bar set "$PLUGIN_ID" launcher "$launcher_path" >/dev/null 2>&1 || true + if ! omarchy bar set "$PLUGIN_ID" launcher "$launcher_path" >/dev/null 2>&1; then + warn "Could not persist the launcher path. Run: omarchy bar set $PLUGIN_ID launcher $launcher_path" + fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-bar-integration.sh` at line 168, Update the launcher persistence command in the install script to stop silently discarding failures from omarchy bar set; retain suppressed normal output, but emit a warning containing the failed command context and launcher path so users can run it manually.
117-117: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winScope the Waybar reload signal to the current user.
waybar_activeusespgrep -u "$(id -u)", but thispkillcall has no user filter. On a multi-user session the installer can signal another user's Waybar process.♻️ Proposed change
- if pkill -SIGUSR2 waybar >/dev/null 2>&1; then + if pkill -SIGUSR2 -u "$(id -u)" -x waybar >/dev/null 2>&1; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-bar-integration.sh` at line 117, Update the Waybar reload command in waybar_active to restrict pkill to the current user, matching the existing pgrep -u "$(id -u)" process check, while preserving the SIGUSR2 signal and success handling.
141-149: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCompare only the managed plugin files instead of the whole directory.
diff -qrcompares every entry of$omarchy_plugin_sourcewith$omarchy_plugin_dir, but onlymanifest.jsonandBarWidget.qmlare installed. If the source directory ever contains another file (README, tests, license), the comparison never matches and each run creates a new.bak.quickdrop.<timestamp>copy. Stale files already present in the target are also never removed.♻️ Proposed change
- mkdir -p "$(dirname "$omarchy_plugin_dir")" - if [[ -d "$omarchy_plugin_dir" ]] && ! diff -qr "$omarchy_plugin_source" "$omarchy_plugin_dir" >/dev/null 2>&1; then - local backup="${omarchy_plugin_dir}.bak.quickdrop.$(date -u +%Y%m%d%H%M%S)" - cp -a "$omarchy_plugin_dir" "$backup" - info "Existing OmarchyBar plugin backed up to $backup" - fi + mkdir -p "$(dirname "$omarchy_plugin_dir")" + local plugin_files=(manifest.json BarWidget.qml) + local changed=0 file + for file in "${plugin_files[@]}"; do + cmp -s "$omarchy_plugin_source/$file" "$omarchy_plugin_dir/$file" || changed=1 + done + if [[ -d "$omarchy_plugin_dir" ]] && (( changed )); then + local backup timestamp + timestamp="$(date -u +%Y%m%d%H%M%S)" + backup="${omarchy_plugin_dir}.bak.quickdrop.${timestamp}" + cp -a "$omarchy_plugin_dir" "$backup" + info "Existing OmarchyBar plugin backed up to $backup" + fiThis variant also resolves the SC2155 hint at line 142.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/install-bar-integration.sh` around lines 141 - 149, Update the backup condition around omarchy_plugin_dir to compare only the managed manifest.json and BarWidget.qml files, rather than recursively diffing the entire directories. Avoid declaring and assigning the backup variable in a single local command so the SC2155 warning is resolved, while preserving the existing backup message and installation behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/LLM_CONTEXT.md`:
- Line 21: Remove the leading space before the Backend entrypoint list marker in
the documentation so it aligns with the other top-level list items and resolves
the MD005 indentation warnings.
- Line 31: Update the status-bar install description in LLM_CONTEXT.md to
identify scripts/quickdrop-waybar as a compatibility alias or copied launcher,
not a compatibility symlink, while preserving the existing description of
scripts/quickdrop-launcher and its configuration behavior.
In `@scripts/install-linux.sh`:
- Around line 158-163: Update write_config in scripts/install-linux.sh (lines
158-163) to read and preserve the existing QUICKDROP_API_BASE_URL from
$config_path when the caller provides no value, instead of overwriting it with
DEFAULT_BASE_URL; continue writing the resulting value normally.
scripts/install-bar-local.sh (lines 7-12) requires no direct change because
preserving the stored value in write_config prevents bar:install from resetting
the local backend.
In `@scripts/install-waybar-module.py`:
- Around line 17-26: Use consistent English user-facing tooltip wording across
both bar adapters: update render_module in scripts/install-waybar-module.py, its
corresponding assertion in scripts/install-waybar-module.test.ts, and
tooltipText in scripts/omarchy-quickdrop/BarWidget.qml (lines 15-16) to the same
English wording.
- Around line 99-118: Update ensure_in_modules_right and its callers to detect
when no modules-right array is found, report that skipped insertion on stderr,
and ensure upsert_module/main do not claim the installation changed
successfully. Handle multi-bar Waybar configurations by applying the module
insertion to every matching modules-right array rather than only the first
pattern match, while preserving existing formatting and duplicate checks.
- Line 80: Update scripts/install-waybar-module.py for Python 3.9 compatibility
by adding the future annotations import and replacing datetime.UTC references
with datetime.timezone.utc, preserving the existing installer behavior.
In `@scripts/install-waybar-module.test.ts`:
- Around line 8-18: Remove the describe.skipIf(!hasPython) guard from the
install-waybar-module.py test suite so it does not silently skip when python3 is
unavailable. Keep the existing patch helper and test definitions unchanged.
In `@scripts/omarchy-quickdrop/BarWidget.qml`:
- Around line 18-22: Update the launcher setting flow in the onPressed handler
to treat launcher as an executable path rather than a command string: rename its
schema label to “Launcher executable” and ensure values such as
quickdrop-launcher --tray are not interpreted as a single executable path, or
provide separate executable and argument settings before calling root.bar.run.
In `@scripts/quickdrop-launcher`:
- Around line 8-19: Update the config loading in the launcher to avoid sourcing
config.env; parse only the QUICKDROP_API_BASE_URL assignment and use its value
without executing other file contents or propagating a nonzero file status.
Normalize the selected api_base_url by removing any trailing slash, matching
scripts/install-linux.sh behavior, while preserving the existing
environment-variable precedence and default URL.
---
Nitpick comments:
In `@scripts/install-bar-integration.sh`:
- Line 168: Update the launcher persistence command in the install script to
stop silently discarding failures from omarchy bar set; retain suppressed normal
output, but emit a warning containing the failed command context and launcher
path so users can run it manually.
- Line 117: Update the Waybar reload command in waybar_active to restrict pkill
to the current user, matching the existing pgrep -u "$(id -u)" process check,
while preserving the SIGUSR2 signal and success handling.
- Around line 141-149: Update the backup condition around omarchy_plugin_dir to
compare only the managed manifest.json and BarWidget.qml files, rather than
recursively diffing the entire directories. Avoid declaring and assigning the
backup variable in a single local command so the SC2155 warning is resolved,
while preserving the existing backup message and installation behavior.
In `@scripts/install-waybar-module.py`:
- Around line 150-157: Update main around config_path.read_text to catch
FileNotFoundError for the configured path and raise SystemExit with a clear
missing-config message, avoiding the traceback while preserving the existing
usage validation and patch flow.
In `@scripts/install-waybar-module.test.ts`:
- Around line 19-31: Add a test case alongside the existing QuickDrop
installation test that passes configuration without a modules-right entry,
exercising ensure_in_modules_right’s early-return path and verifying the
resulting configuration contains no visible QuickDrop module.
In `@src/server/index.ts`:
- Around line 60-78: Optionally consolidate the six Linux asset registrations
into a single route-to-file table and iterate over it to call sendScriptFile.
Preserve every existing route, including the backward-compatible
quickdrop-waybar endpoint and all launcher, integration, Python, manifest, and
QML asset mappings.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0129d595-ff1b-4530-8e24-da0ebcf51e81
📒 Files selected for processing (19)
DockerfileREADME.mddocs/LLM_CONTEXT.mddocs/mvp.mdpackage.jsonscripts/install-bar-integration.shscripts/install-bar-integration.test.tsscripts/install-bar-local.shscripts/install-desktop.tsscripts/install-linux.shscripts/install-linux.test.tsscripts/install-waybar-module.pyscripts/install-waybar-module.test.tsscripts/install-waybar-module.tsscripts/omarchy-quickdrop/BarWidget.qmlscripts/omarchy-quickdrop/manifest.jsonscripts/quickdrop-launchersrc/server/index.test.tssrc/server/index.ts
💤 Files with no reviewable changes (1)
- scripts/install-waybar-module.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 19 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
Owner
Author
|
Apliquei o feedback dos reviews no commit
Validação atualizada: |
There was a problem hiding this comment.
All reported issues were addressed across 13 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Owner
Author
|
Validei e tratei os dois novos apontamentos no commit
Validação: typecheck e todos os 71 testes passando. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumo
QUICKDROP_BAR=auto|omarchy|waybar|both|nonequickdrop-launcher, independente da barra, mantendoquickdrop-waybare seu endpoint como compatibilidadequickdrop.barusando o contrato oficialbar-widgetValidação
bun run typecheckbun test— 64 testes passandobun run desktop:build:webomarchy plugin validate scripts/omarchy-quickdropbash -nnos scripts Linuxdocker build -t quickdrop:omarchy-bar-test .Observação
cargo test --manifest-path src-tauri/Cargo.tomlnão foi executado porquecargonão está instalado neste ambiente; não houve alteração no código Rust.Summary by cubic
Adds OmarchyBar and Waybar integrations behind a shared
quickdrop-launcher, replacing the Waybar-only launcher. The launcher readsQUICKDROP_API_BASE_URLfrom env or~/.config/quickdrop/config.envwithout executing the file and now normalizes quoted values; existing Waybar installs continue to work via the/linux/quickdrop-waybaralias.quickdrop.bar) using thebar-widgetcontract (QML); it launchesquickdrop-launcher.quickdrop-waybarwithquickdrop-launcher(new lock file name) and trims trailing slashes from the backend URL.custom/quickdropand writes a JSONC backup.install-linux.shandinstall-bar-integration.shwithQUICKDROP_BAR=auto|omarchy|waybar|both|none; addsscripts/install-bar-local.shandbun run bar:install./linux/quickdrop-launcher,/linux/install-bar-integration,/linux/install-waybar-module.py, and Omarchy assets; Docker image ships them.Review focus
QUICKDROP_BARoverride ininstall-bar-integration.shandinstall-linux.sh.install-waybar-module.py.quickdrop-launcher./linux/quickdrop-waybaralias.config.env, safely normalizes quotes, and installers modify only user-owned paths.Rollout/Migration
bun run bar:install(orwaybar:install/omarchy-bar:install).quickdrop-waybardirectly, switch toquickdrop-launcher.Written for commit 774adad. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation