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: add tag dispacther and window rule #6211

Merged
merged 12 commits into from
May 28, 2024
Merged

Conversation

rtgiskard
Copy link
Contributor

@rtgiskard rtgiskard commented May 23, 2024

Describe your PR, what does it fix/add?

this resolves: #6079

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

  1. the tag rule will update tags which affect the rule matched by tag, may be a rule rescan after tag applied
  2. dynamic tag shouldn't be permernant
  3. tag matched rule not get executed right after tag is applied by dispatcher
  4. hyprctl bash/fish/zsh completion is not synced yet, just not sure how to update

Is it ready for merging, or does it need work?

Likely to be ready except for the above note

@rtgiskard
Copy link
Contributor Author

I'm using clangd v18.1.3, while the archlinux default clang is v17.0.6,

looks like the new clang-format does not agree with the old one, which cause the failure of code style check

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

one thing stands out to me: you apply a tag in dynamic but dont clear them before. This means a window won't lose a tag when it stops matching a dynamic prop (e.g. windowrulev2 = tag:cock, floating:1 -> toggling float will permanently add the tag cock)

I think a reasonable approach is to just make a struct SWindowTag which has a bool dynamic, and clear dynamic tags before reloading dynamic rules

@vaxerski
Copy link
Member

tags added via a dispatcher should be permanent though so those would not be tagged as dynamic I guess

@rtgiskard
Copy link
Contributor Author

Well, I'll check later, I need to have a late-night snack. 😂

@rtgiskard
Copy link
Contributor Author

rtgiskard commented May 24, 2024

Dynamic tag is implemented with a * suffix compared to static tag via dispatcher.

Tag rule will queue another rescan for window rules, while tag from tag will not be queued for another rescan to avoid infinite recursion like this:

windowrulev2 = tag code, tag:cpp
windowrulev2 = opacity 0.7, tag:code

though, it would be ok with this if allow tag from tag recursion:

windowrulev2 = tag +code, tag:cpp
windowrulev2 = opacity 0.7, tag:code    #4

With current implementation, #4 will not execute in the first place.


fixed, no rescan with latest implementation

src/desktop/Window.cpp Outdated Show resolved Hide resolved
src/desktop/Window.cpp Outdated Show resolved Hide resolved
@rtgiskard rtgiskard requested a review from vaxerski May 24, 2024 10:15
@rtgiskard rtgiskard force-pushed the dev.tag branch 2 times, most recently from deef8ad to c7a5828 Compare May 25, 2024 11:15
@rtgiskard
Copy link
Contributor Author

rtgiskard commented May 25, 2024

Now tags works perfect, seems to be ready

@rtgiskard
Copy link
Contributor Author

Now tags works perfect, seems to be ready

@rtgiskard
Copy link
Contributor Author

Doc for tag is updated here: hyprwm/hyprland-wiki#660

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

sum style nits, rest ok

src/config/ConfigManager.cpp Outdated Show resolved Hide resolved
src/helpers/TagKeeper.hpp Outdated Show resolved Hide resolved
src/helpers/TagKeeper.hpp Outdated Show resolved Hide resolved
@rtgiskard rtgiskard requested a review from vaxerski May 27, 2024 10:12
@vaxerski
Copy link
Member

class CTagKeeper but file TagKeeper (just like everything else) and then gtg

@rtgiskard
Copy link
Contributor Author

class CTagKeeper but file TagKeeper (just like everything else) and then gtg

Well, mistake. By the way, would you like to try the bleeding-edge cpp modules?

@vaxerski
Copy link
Member

nope, time will come for them in the future thugh prolly

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

lgtm

@vaxerski vaxerski merged commit ebf2587 into hyprwm:main May 28, 2024
10 checks passed
@rtgiskard rtgiskard deleted the dev.tag branch May 29, 2024 03:08
cdubthecoolcat pushed a commit to cdubthecoolcat/Hyprland that referenced this pull request May 29, 2024
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.

add a tag property to window, allow update tag with dispatch, and match with tag
2 participants