feat: waybar app launcher module #1557
Conversation
Added support for passing extra args to rofi. The idea was to use `-click-to-exit` option to be able to control app menu without keyboard fully. But at the moment `-click-to-exit` is unsupported on wayland davatorium/rofi#2158
📝 WalkthroughWalkthroughUpdates rofilaunch.sh to consume one positional argument per action branch and pass remaining args to rofi; consolidates border CSS properties into a shorthand in Waybar styles; and adds a new Waybar custom application launcher module configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🤖 Fix all issues with AI agents
In `@Configs/.local/lib/hyde/rofilaunch.sh`:
- Around line 65-66: The array append currently uses unquoted positional
parameters (rofi_args+=($@)), which causes word splitting; change the append so
each original argument is preserved as its own array element by using the quoted
form of the positional parameters when appending to rofi_args (i.e., append
using the quoted $@ form) so arguments with spaces like -theme "My Theme" are
not split.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Configs/.local/lib/hyde/rofilaunch.shConfigs/.local/share/waybar/modules/custom-applauncher.jsoncConfigs/.local/share/waybar/styles/defaults.css
🧰 Additional context used
🧬 Code graph analysis (1)
Configs/.local/share/waybar/modules/custom-applauncher.jsonc (1)
Configs/.local/lib/hyde/waybar.py (3)
update_icon_size(833-872)generate_includes(1193-1225)update_config(1228-1231)
🪛 Biome (2.1.2)
Configs/.local/share/waybar/styles/defaults.css
[error] 242-242: Unexpected value or character.
Expected one of:
(parse)
[error] 246-246: Unexpected value or character.
Expected one of:
(parse)
[error] 244-245: Unknown type selector is not allowed.
See MDN web docs for more details.
Consider replacing the unknown type selector with valid one.
(lint/correctness/noUnknownTypeSelector)
[error] 245-245: Unknown type selector is not allowed.
See MDN web docs for more details.
Consider replacing the unknown type selector with valid one.
(lint/correctness/noUnknownTypeSelector)
🪛 Shellcheck (0.11.0)
Configs/.local/lib/hyde/rofilaunch.sh
[warning] 65-65: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
🔇 Additional comments (4)
Configs/.local/share/waybar/styles/defaults.css (1)
238-247: LGTM! Border consolidation improves maintainability.The shorthand
borderproperty correctly consolidates the previous separateborder-widthandborder-colordeclarations. This addresses the PR objective of fixing intermittent missing borders.Note: The static analysis warnings about "unexpected value" and "unknown type selector" are false positives—Biome is a web CSS linter and doesn't recognize GTK CSS variable syntax (
@main-fg) or GTK-specific selectors likewindow.popup decoration.Configs/.local/share/waybar/modules/custom-applauncher.jsonc (1)
1-9: Module configuration looks good.The structure is valid and integrates well with the existing Waybar module discovery system. The
hyde-shell rofilaunch d -click-to-exitinvocation aligns with the updated argument handling inrofilaunch.sh.One consideration: since
-click-to-exitis unsupported on Wayland (as noted in the PR description), users on Wayland won't get the expected click-to-dismiss behavior. This is acceptable given the upstream limitation, but you might consider documenting this in the tooltip or module comments for discoverability.Configs/.local/lib/hyde/rofilaunch.sh (2)
55-62: Verify intended behavior for the default caseshift.When the script is called without a recognized mode (e.g.,
rofilaunch -click-to-exitinstead ofrofilaunch d -click-to-exit), theshifton line 61 will consume the first argument, causing it to be lost before appending torofi_argson line 65.If the expectation is that users must always specify a mode explicitly, this is fine. Otherwise, consider guarding the shift:
Optional: conditional shift for default case
*) r_mode="drun" ROFI_LAUNCH_DRUN_STYLE="${ROFI_LAUNCH_DRUN_STYLE:-$ROFI_LAUNCH_STYLE}" rofi_args+=("${ROFI_LAUNCH_DRUN_ARGS[@]:-}") rofi_args+=("-run-command" "app2unit.sh --fuzzel-compat -- {cmd}") rofi_config="${ROFI_LAUNCH_DRUN_STYLE:-$rofi_config}" - shift + [[ -n "$1" ]] && shift ;;
20-46: LGTM! Shift operations correctly consume mode arguments.The
shiftadditions after each case enable passing extra arguments (like-click-to-exit) through to rofi, which is the intended behavior for the new Waybar module.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Configs/.local/lib/hyde/rofilaunch.sh (1)
55-62: Bug:shiftin default case discards arguments that should be passed through.The default case handles two scenarios:
- No arguments provided (run drun mode)
- First argument is not a recognized mode selector
In scenario 2, the user's first argument (e.g.,
-click-to-exit) is consumed byshiftand lost. For example,rofilaunch.sh -click-to-exitwill drop the flag entirely.Proposed fix
Remove the
shiftfrom the default case since no mode argument was consumed:*) r_mode="drun" ROFI_LAUNCH_DRUN_STYLE="${ROFI_LAUNCH_DRUN_STYLE:-$ROFI_LAUNCH_STYLE}" rofi_args+=("${ROFI_LAUNCH_DRUN_ARGS[@]:-}") rofi_args+=("-run-command" "app2unit.sh --fuzzel-compat -- {cmd}") rofi_config="${ROFI_LAUNCH_DRUN_STYLE:-$rofi_config}" - shift ;;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Configs/.local/lib/hyde/rofilaunch.sh
🔇 Additional comments (2)
Configs/.local/lib/hyde/rofilaunch.sh (2)
20-46: LGTM!The
shiftadditions in the recognized mode cases correctly consume the mode selector argument, allowing subsequent positional parameters to be forwarded to rofi.
65-66: LGTM!The quoted
"$@"correctly preserves argument boundaries, addressing the previous review feedback.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
But we are using this version of rofi now. the Wayland fork has already been merged. Will it break if unsupported? |
|
Nope, it does not break anything. Option just dont work silently. This is why I decided to leave it (hopes it will work some day) |
|
can you name it 'app-launcher'? Also maybe check #1557 (review) . |
|
@kRHYME7 renamed, review comment fixed |
2 participants
Pull Request
Description
Feature
This PR is relateted to #1551
Adds simple waybar module which opens rofi apps menu with a click
Also added support for passing extra args to rofi via
hyde-shell rofilaunch.The idea was to use
-click-to-exitoption to be able to control rofi menu without keyboard fully.But at the moment
-click-to-exitis unsupported on wayland davatorium/rofi#2158Fixes
Change styles for tooltip & menu decoration because in certain (almist random) cases there were no border for popups. Changed styles looks stable and do not show this random border miss.
Type of change
Checklist
Summary by CodeRabbit
New Features
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.