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

feat: Niri Service #54

Merged
merged 14 commits into from
Dec 6, 2024
Merged

feat: Niri Service #54

merged 14 commits into from
Dec 6, 2024

Conversation

regenman
Copy link
Contributor

@regenman regenman commented Dec 4, 2024

This PR adds a Niri service, including a sample config.
Based on the hyprland service.

@linkfrg linkfrg added the enhancement New feature or request label Dec 5, 2024
@linkfrg
Copy link
Owner

linkfrg commented Dec 5, 2024

I think creating a separate example bar for Niri leads to code repetition and makes these examples harder to maintain

How about adding a check? If running on Hyprland, display the hyprland workspaces, if running on Niri, display the Niri widgets accordingly

@regenman
Copy link
Contributor Author

regenman commented Dec 5, 2024

I think creating a separate example bar for Niri leads to code repetition and makes these examples harder to maintain

I completely agree.

How about adding a check? If running on Hyprland, display the hyprland workspaces, if running on Niri, display the Niri widgets accordingly

Sounds good. What do you think would be a sensible way to check this, test the existence of NIRI_SOCKET / HYPR_SOCKET_DIR ?

@linkfrg
Copy link
Owner

linkfrg commented Dec 5, 2024

Sounds good. What do you think would be a sensible way to check this, test the existence of NIRI_SOCKET / HYPR_SOCKET_DIR ?

Hmmm, or add new properties to Hyprland Service and Niri Service like is_available, or catch exceptions (HyprlandIPCNotFoundError, NiriIPCNotFoundError)...

I'm more inclined to the first option

  1. I will add the is_available property to the Hyprland Service in the main branch
  2. Merge this change from the main to this PR
  3. Add new property to the Niri Service here
  4. Add a check to the bar example

EDIT: Better leave HyprlandIPCNotFoundError and NiriIPCNotFoundError, but now they shouldn't be raised during service initialization. Only on requesting operations with socket like switch_to_workspace(), send_command() and etc.
Listening to socket should be skipped if is_available is False

…or only on manual operations with the socket and not during service initialization
@regenman
Copy link
Contributor Author

regenman commented Dec 6, 2024

4. Add a check to the bar example

I'm a bit stuck on how to structure the example, in a way that minimizes code duplication. My apologies for being a bit inexperienced with OOP.
In the case of my "working prototype" config.py for niri, I'm passing down "monitor_name" as function parameter all the way from bar()->left()->workspaces()-> .. , reason being that niri indexes workspaces separately per monitor. Without this, with a dual monitor setup, your list of workspaces would be "1 1 2 2 3 3 .." . But passing a "monitor_name" parameter wouldn't necessarily be a useful addition to the corresponding hyprland functions, due to the differences in how it handles workspaces.
Should I just make the whole example a class instead, if I understand it correctly it would allow access to variables across functions?
Any hints or ideas would be appreciated.

@linkfrg
Copy link
Owner

linkfrg commented Dec 6, 2024

4. Add a check to the bar example

I'm a bit stuck on how to structure the example, in a way that minimizes code duplication. My apologies for being a bit inexperienced with OOP.

In the case of my "working prototype" config.py for niri, I'm passing down "monitor_name" as function parameter all the way from bar()->left()->workspaces()-> .. , reason being that niri indexes workspaces separately per monitor. Without this, with a dual monitor setup, your list of workspaces would be "1 1 2 2 3 3 .." . But passing a "monitor_name" parameter wouldn't necessarily be a useful addition to the corresponding hyprland functions, due to the differences in how it handles workspaces.

Should I just make the whole example a class instead, if I understand it correctly it would allow access to variables across functions?

Any hints or ideas would be appreciated.

Just make separate functions for niri (move them from the current bar-niri). And add a check in the workspaces() function using the newly added is_available() property. If running on hypland, return hyprland_workspaces() (you need to move hyprland workspaces to separate function), if on niri, returnniri_workspaces(), else return empty Widget.Box

@regenman
Copy link
Contributor Author

regenman commented Dec 6, 2024

Thank you.
I think I've arrived at a working example. Tested under niri and hyprland, also seems to work as expected when neither of those are available.

@linkfrg
Copy link
Owner

linkfrg commented Dec 6, 2024

Okay, looks good

Is it ready for merging or you're planning to add additional functionality or fixes?

@regenman
Copy link
Contributor Author

regenman commented Dec 6, 2024

Thank you for the fixups.
I'm not planning to add more functionality at this point. In my personal config I've added a simple power menu, but it's unrelated to niri and could perhaps go as a separate PR. I'll mark this as ready for review.

@regenman regenman marked this pull request as ready for review December 6, 2024 18:52
@linkfrg
Copy link
Owner

linkfrg commented Dec 6, 2024

Thanks!
Merging...

@linkfrg linkfrg merged commit 9e3ec7a into linkfrg:main Dec 6, 2024
3 checks passed
@linkfrg linkfrg linked an issue Dec 6, 2024 that may be closed by this pull request
@regenman regenman deleted the niri-ipc branch December 8, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Niri IPC support (Niri Service)
2 participants