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 Sway support as separate feature #138

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

markstos
Copy link

@markstos markstos commented Feb 17, 2022

This an alternate approach to supporting Sway, competing with the PR #122

Old Approach -- Shared Feature

In #122, both Sway and i3 were supported as the the same compile-time feature. This was done by switching from an i3-specific library to "swayipc", which supports both window managers.

This maximized shared code, but in practice was difficult to get merged because the Sway developer didn't test on i3, where it turned to break support. But the i3 developer didn't have Sway installed. The situation meant that in practice it was hard to make changes to support either WM without a risk of breaking the other.

New Approach -- Seperate Feature

In the new approach, the i3 code is not really touched at all, and should therefore continue to work just as it did before.

Sway is implemented as a separate feature. Now code for either WM can be updated without a risk of breaking the other.

One challenge with the new approach is each is implemented as a compile-time feature.

Packagers who need to support both window-managers will need to publish two packages, like wmfocus-i3, wmfocus-sway. Depending on which is selected, the installed wmfocus will work with one or the the other.

I'm a beginning Rust developer and don't expect to be able to support them both at a runtime with this approach, but maybe someone can build on what I've done to make that work.

Both approaches draw the windows with X11 not Wayland. Having the Sway feature draw the the windows with Wayland is another feature which could be added down the road.

Currently only either i3 or Sway can be selected as a compile-time
feature with one of:

  cargo build --features i3 wmfocus
  cargo build --features sway wmfocus

Based on a first draft by @JayceFayne
@markstos
Copy link
Author

To test this on Sway, I used cargo build --features sway --release && ./target/release/wmfocus.

"Worked for me".

@mightyiam
Copy link

I'd love to try this on sway. Has it been reviewed?

@markstos
Copy link
Author

@mightyiam You are welcome to test it.

@markstos
Copy link
Author

I have a question which concerns this also #146: Does the i3 functionality select containers or just windows? I noticed in the animation included in the README that there's no hint for parent containers, just for windows. It seems the Sway implementation has always hinted parent containers as well.

A lot of time that's distracting and requires some extra visual cues to understand what is going on (that's what #146 addresses), but other times it's useful. For example, if you have a collection of browser-type "tabs" or a collection of terminal "tabs", you can directly switch focus to this parent container, then kill the parent and the child windows in one go. Or sometimes I want to clear out all the windows on a desktop, so I select the root hint for the workspace and kill all the windows at once.

On the other hand, since switching to windows is by far the common use, it's not that hard to select a collection of tabs by switching to focus to any of the tabs and then using a key binding to select the parent container.

@matejdro
Copy link

matejdro commented Feb 21, 2024

Thanks. This mostly works, but it seems that it generates duplicate letters on all windows (Konsole on the screenshot is native Wayland, Discord is XWayland).

wmfocus

Navigating to either letter works.

@markstos
Copy link
Author

@matejdro If you have enough windows, there will start to be more than one letter, or if you have containers with apps with them, that also causes extra letters. In some branch I added visual offsets so that letters for windows don't appear at the same height as letters for containers.

This project seems pretty dead, though. I submitted this PR more than two years ago.

Instead, I recommend the sway-easyfocus project.

It needs multi-monitor support but is otherwise good. I started to add multi-monitor support but got stalled due to lack of Rust experience:

https://github.com/edzdez/sway-easyfocus

Whatever issue you are happening, you may have better luck getting it resolved in that project.

@svenstaro
Copy link
Owner

This project is not dead. I'm still actively interested in getting support for other WMs in here. I maintain it currently for the X11 variant (which is use daily). Are you interested in working on the PR?

@markstos
Copy link
Author

@svenstaro Working on the PR? I completed it and turned it for review over two years ago. That was after I submitted a first PR for Sway support, which was never merged, but also never closed: #122

I also submitted a PR for offsetting the hints:
#146

About a year later, you commented on that "looking at it now!". I promptly addressed the feedback, then then PR hasn't received a follow-up for over a year either.

I've started contributing to the sway-easyfocus project instead, which has more active maintenance.

If you are actively interested in supporting other WMs, one way to express that interest would be merge the PRs that people submit for supporting other WMs.

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.

None yet

4 participants