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

river: Allow applying CSD based on window titles #424

Merged
merged 1 commit into from
Sep 7, 2021

Conversation

ThreeFx
Copy link
Contributor

@ThreeFx ThreeFx commented Sep 6, 2021

This extends the csd-filter-add command to allow matching on window
titles as well, using a csd-filter-add kind pattern syntax. The
following kinds are supported:

  • title, which matches window titles
  • app-id, which matches app ids

Only exact matches are considered.

As an example following configuration applies client-side decorations to
all windows with the title 'asdf with spaces'.

riverctl csd-filter-add title 'asdf with spaces'

This commit makes the *-filter-add consistent over floats and csds.

river/Config.zig Outdated Show resolved Hide resolved
@ThreeFx ThreeFx marked this pull request as draft September 6, 2021 13:41
@ThreeFx
Copy link
Contributor Author

ThreeFx commented Sep 6, 2021

I'm not clear on what semantics we want to have for csd-filter-add. When should a view have client-side decorations? My current understanding of the current implementation is:

  • When the app-id filter matches the view's toplevel

What semantics do we want when also including the title? Currently, I think I've implemented it the following way:

  • When the app-id or title matches the view itself

I don't have enough experience with Wayland to judge whether this is ok or not.


Separate point:

Adding/removing filters at runtime works for both app-id filters as well as title filters, but as is the case with floating, this currently does not handle title updates. Since csds are applied for existing and new windows, should we handle title updates in this case?

@Leon-Plickat
Copy link
Member

Only use CSD if the filter matches is the right thing here. This filter exists mostly for cosmetics.

I don't think we should care about updating titles. Eventually all this will be done by an external program, which can deal with that sort of thing.

Copy link
Member

@ifreund ifreund left a comment

Choose a reason for hiding this comment

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

For any given view, the view's app-id and title are exactly the app-id and title of the view's backing xdg-toplevel. River's View struct exists purely to abstract over xwayland surfaces and normal xdg-toplevels. So, the way you've implemented that part looks good to me.

With regards to title updates, I'm not sure if it makes sense to handle them or not. Therefore let's go with the simpler option of not handling them for now and see if anyone comes up with a motivating use-case for doing so.

doc/riverctl.1.scd Outdated Show resolved Hide resolved
This extends the `csd-filter-add` command to allow matching on window
titles as well, using a `csd-filter-add kind pattern` syntax. The
following kinds are supported:

  * `title`, which matches window titles
  * `app-id`, which matches app ids

Only exact matches are considered.

As an example following configuration applies client-side decorations to
all windows with the title 'asdf with spaces'.

    riverctl csd-filter-add title 'asdf with spaces'
@ThreeFx ThreeFx marked this pull request as ready for review September 6, 2021 17:43
@ThreeFx ThreeFx requested a review from ifreund September 6, 2021 17:43
@ThreeFx
Copy link
Contributor Author

ThreeFx commented Sep 6, 2021

Alright, then this is ready for review. I've added a short note to riverctl(1) stating that we do not account for title updates.

I haven't tested this though, because I haven't got the slightest clue which apps use csd and which don't. Let me know which ones are good test candidates.

@ifreund
Copy link
Member

ifreund commented Sep 6, 2021

I haven't tested this though, because I haven't got the slightest clue which apps use csd and which don't. Let me know which ones are good test candidates.

All apps theoretically support CSD thanks to GNOME's influence on early wayland developement.

I usually just test this stuff with foot, you should see a title bar appear if you run riverctl csd-filter-add app-id foot.

@ThreeFx
Copy link
Contributor Author

ThreeFx commented Sep 6, 2021

Ah indeed, thanks! I've tested it using my terminal (Alacritty) with both app-id and title - works like a charm.

Copy link
Member

@ifreund ifreund left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@ifreund ifreund merged commit 5f6428b into riverwm:master Sep 7, 2021
@ThreeFx ThreeFx deleted the add-title-filtering-to-csd branch September 8, 2021 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants