Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support to hyprlandctl #102

Merged
merged 5 commits into from
Nov 8, 2023
Merged

add support to hyprlandctl #102

merged 5 commits into from
Nov 8, 2023

Conversation

gouvinb
Copy link
Contributor

@gouvinb gouvinb commented Nov 5, 2023

Description

This pull request adds the wm flag to the nwg-drawer command to launch a program with the swaymsg exec or hyprctl dispatch exec command.

⚠️ BREAKING CHANGE: '-swaymsg' removed, use '-wm sway' instead

Reference

This pull request refers to this comment (#101)

Instructions

To use this new feature, simply add the -wm <current wm name> argument to the nwg-drawer command, or set the XDG_CURRENT_DESKTOP environment variable to "sway", "hypr" or "hyprland".

Example

Before

rubymine # without -term, -swaymsg and without XDG_CURRENT_DESKTOP=sway

After

swaymsg exec firefox # with `-wm sway` or with XDG_CURRENT_DESKTOP=sway

or

hyprctl dispact exec firefox # with `-wm hyprland` or with XDG_CURRENT_DESKTOP=hyprland

or

hyprctl dispact exec firefox # with `-wm hypr` or with XDG_CURRENT_DESKTOP=hypr

Note

I don't use Hypr or Hyprland (yet). So this PR must be tested.

BREAKING CHANGE: '-swaymsg' removed, use '-wm sway' instead
Copy link
Owner

@nwg-piotr nwg-piotr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be hyprctl, hot hyprlandctl, e.g.: hyprctl dispatch exec alacritty.

Copy link
Owner

@nwg-piotr nwg-piotr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work well on Hyprland. Would you mind adding the prefix to the output in https://github.com/gouvinb/nwg-drawer/blob/808fd032a0ffd675b4cf20b221f3f8c9702fe56c/tools.go#L621 ?

@nwg-piotr
Copy link
Owner

Thanks! I see hardly anything on the phone. Will test tonight, when I'm back home.

@gouvinb
Copy link
Contributor Author

gouvinb commented Nov 6, 2023

Wait, I've caught a bug with this kind of command: /opt/google/chrome/google-chrome --profile-directory=Default --app-id=xxx.

Typical shell-supported command sets:

- `cmd`
- `cmd --arg`
- `cmd --arg=value`
- `ENV_VAR="value" cmd`
- `ENV_VAR="value" cmd --arg`
- `ENV_VAR="value" cmd --arg=value`

The listed commands above are now better managed.

Initially, the command types `cmd --arg=value` or `ENV_VAR="value" cmd --arg=value` were not correctly parsed. After finding a solution to this problem, I conducted further tests and discovered cases that were not currently handled. These cases are as follows:

- `cmd "arg with space"`
- `ENV_VAR="value with space" cmd`
@gouvinb
Copy link
Contributor Author

gouvinb commented Nov 6, 2023

I reworked command building.

Typical shell-supported command sets:

  • cmd
  • cmd --arg
  • cmd --arg=value
  • ENV_VAR="value" cmd
  • ENV_VAR="value" cmd --arg
  • ENV_VAR="value" cmd --arg=value

The listed commands above are now better managed.

Initially, the command types cmd --arg=value or ENV_VAR="value" cmd --arg=value were not correctly parsed. After finding a solution to this problem, I conducted further tests and discovered cases that were not currently handled. These cases are as follows:

  • cmd "arg with space"
  • ENV_VAR="value with space" cmd

Copy link
Owner

@nwg-piotr nwg-piotr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running apps with go is insanely complicated and always makes me miss the good, old python. I'm afraid we will never have a code that properly starts everything with all possible env vars and arguments.

However:

main.go, line 128 is ambiguous

log.Warn("nwg-drawer support only 'swaymsg' or 'hyprland'.")

Consider something like this:

log.Warn("-wm argument supports only sway or hyprland string")

main.go, line 152, the description is not true

var wm = flag.String("wm", defaultStringIfBlank(os.Getenv("XDG_CURRENT_DESKTOP"), ""), "Use swaymsg (with 'sway' argument) or hyprctl (with 'hypr' or 'hyprland')")

We only seem to support the hyprland argument, but not hypr.

@gouvinb
Copy link
Contributor Author

gouvinb commented Nov 7, 2023

Running apps with go is insanely complicated and always makes me miss the good, old python.

The approach with the Go language is a bit too low-level.

I think that most cases are covered and that the remaining 2 cases represent only a minority of situations.

Personally, I've never had a case like this; I had to invent them myself to check whether they worked or not.

@nwg-piotr
Copy link
Owner

All right. Let's find it ready.

@nwg-piotr nwg-piotr merged commit 4446178 into nwg-piotr:main Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants