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

Change window swallowing to use rules. #151

Closed
wants to merge 0 commits into from

Conversation

kodesoul
Copy link

Feel free to ignore this PR if it goes against default structures.

This PR replaces the default behaviour of setting swallow rules in the theme with setting them with awful.rules.
I'm not sure if this is better but to me seems to make more sense.

@kodesoul kodesoul requested a review from Nooo37 as a code owner January 10, 2022 09:15
@eylles
Copy link
Contributor

eylles commented Jan 11, 2022

what happens when you turn the swallow filter off with false?, i added that horrible juggling with the filter var because using booleans with lua in the way it is used in here will not act at all in the expected way.

@kodesoul
Copy link
Author

I've done some testing and it seems to work just fine. I guess since it's only one switch i.e
window_swallowing_activated toggling works fine. In terms of setting the swallow rules
as client properties it seems that awful is handling any possible funkiness.

@Nooo37
Copy link
Member

Nooo37 commented Jan 23, 2022

I really like that idea. Now that you PR it, it seems to be almost obvious that it should be that way. Good job! One thing I don't like though is how one has to set swallow_parent to true for all clients in order to get "the default behavior". I think it should just swallow everything by default and then the user can add exceptions. We used to have dont_swallow(filter list) but that's obviously not a perfect naming scheme either. Maybe swallow_exception would fit better?

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.

3 participants