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

Improve criteria handling #1149

Merged
merged 1 commit into from
Apr 6, 2017
Merged

Improve criteria handling #1149

merged 1 commit into from
Apr 6, 2017

Conversation

4e554c4c
Copy link
Contributor

@4e554c4c 4e554c4c commented Apr 5, 2017

This commit changes how commands decide what container to act on.
Commands get the current container though current_container, a global
defined in sway/commands.c. If a criteria is given before a command,
then the following command will be run once for every container the
criteria matches with a reference to the matching container in
'current_container'. Commands should use this instead of
get_focused_container() from now on.

This commit also fixes a few (minor) mistakes made in implementing marks
such as non-escaped arrows in sway(5) and calling the "mark" command
"floating" by accident. It also cleans up criteria.c in a few places.

I have tested the changed commands and things seem to work.

@ddevault
Copy link
Contributor

ddevault commented Apr 5, 2017

I was just thinking about this the other day, thanks for working on it!

sway/commands.c Outdated
@@ -43,6 +43,8 @@ struct cmd_handler {

int sp_index = 0;

swayc_t *current_container;
Copy link
Contributor

Choose a reason for hiding this comment

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

Initialize to NULL

sway/criteria.c Outdated
@@ -373,3 +373,25 @@ list_t *criteria_for(swayc_t *cont) {
}
return matches;
}

void container_for(swayc_t *container, list_t *tokens, list_t *list) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use container_map instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to use container map, but that only takes two arguments when I would need three: tokens, container and final list. I could solve that by passing a (container list,tokens list) struct or use a global, but both felt somewhat jurry-rigged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Use a struct.

sway/sway.5.txt Outdated
Criteria may be used with either the **for_window** or **assign** commands to
specify operations to perform on new views. A criteria may also be used to
perform specific commands (ones that normally act upon one window) on all views
that match that criteria. Here are some ways to use criteria in this way.
Copy link
Contributor

@ddevault ddevault Apr 5, 2017

Choose a reason for hiding this comment

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

s/Here are some ways to use criteria in this way/For example:/

@ddevault
Copy link
Contributor

ddevault commented Apr 5, 2017

The switch to current_container is a good idea, but it almost certain to create some bugs. Please carefully check to make sure you've found every case.

Looks good overall. Let me know when you've fixed the issues you mentioned and I'll take a closer look.

This commit changes how commands decide what container to act on.
Commands get the current container though `current_container`, a global
defined in sway/commands.c. If a criteria is given before a command,
then the following command will be run once for every container the
criteria matches with a reference to the matching container in
'current_container'. Commands should use this instead of
`get_focused_container()` from now on.

This commit also fixes a few (minor) mistakes made in implementing marks
such as non-escaped arrows in sway(5) and calling the "mark" command
"floating" by accident. It also cleans up `criteria.c` in a few places.
@4e554c4c
Copy link
Contributor Author

4e554c4c commented Apr 6, 2017

I've gone back over the code and fixed a few things. Turns out the "title matching bugs" was just a consequence of how I was testing with swaymsg. One of the things that might not be 100% wanted is moving (non-flotaing)containers always focuses them, I can work on this if you'd like.

@ddevault
Copy link
Contributor

ddevault commented Apr 6, 2017

Please do, yes. But we can address that separately. Thanks!

@ddevault ddevault merged commit 3f40b61 into swaywm:master Apr 6, 2017
@4e554c4c 4e554c4c deleted the criteria branch April 6, 2017 17:23
@ddevault ddevault mentioned this pull request Apr 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants