Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add apply_superset_style to calculate the maximum values for all criteria styles #68

Merged
merged 13 commits into from
Oct 10, 2018

Conversation

vilhalmer
Copy link
Collaborator

@vilhalmer vilhalmer commented Sep 30, 2018

This replaces global_criteria as the way to find out about notification styles in the general case, rather than for a specific notification. This is useful for two things that aren't currently possible to do accurately:

  • Sizing the layer surface (what's the widest notification we can display?)
  • Figuring out which capabilities to advertise (are they set for at least one criteria?)

This doesn't directly fix any issues, but was necessary for #46, #47, and #48, and will improve the fix for #55.

The main behavior change here is that we're now advertising DBus capabilities based on whether any of the criteria are using them, rather than just the default section. This means that you can now, for example, turn off markup globally but reenable it for a specific type of notification. Before, you could only do the opposite.

I've eliminated all but one use of global_criteria. It will be able to go away entirely after #66, because the stacking mechanism can be used to make the hidden criteria no longer a special case (yay!).

The one tricky part of this was the "maximum" value for format: a collection of all of the specifiers that appear in at least one style. I added format validation to make it a little easier (by knowing what the maximum length of that collection can be).

Finally, I took the opportunity of needing to generate the "superstyle" on each reconfig to unify config loading during startup with that of reload_config. The argc and argv are now stored on the mako_state to make that easier.

config.c Outdated
@@ -13,10 +13,14 @@
#include "types.h"
#include "wlr-layer-shell-unstable-v1-client-protocol.h"

#define max(a, b) (((a) > (b)) ? (a) : (b))
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not a fan of macros. This can be just a normal function, which will be automatically inlined by the compiler.

config.c Outdated
@@ -79,8 +84,12 @@ void init_empty_style(struct mako_style *style) {
}

void finish_style(struct mako_style *style) {
free(style->font);
free(style->format);
if (style->spec.font) {
Copy link
Owner

Choose a reason for hiding this comment

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

No need for these ifs. free(NULL) is valid and does nothing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're absolutely right, not sure why I suddenly felt the need to add these.

config.c Show resolved Hide resolved
config.c Outdated
target->spec.format = true;

if (target->format) {
free(target->format);
Copy link
Owner

Choose a reason for hiding this comment

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

No need for these ifs. free(NULL) is valid and does nothing.

criteria.c Outdated
@@ -134,6 +135,9 @@ bool parse_criteria(const char *string, struct mako_criteria *criteria) {
++token_location;
}
break;
default:
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm. I'm not a fan of adding default cases since most compilers will emit a warning when an enum case isn't handled.

include/types.h Outdated
@@ -25,6 +25,10 @@ struct mako_directional {
};

bool parse_directional(const char *string, struct mako_directional *out);

// List of specifier characters that can appear in a format string.
static const char VALID_FORMAT_SPECIFIERS[] = "%asbht";
Copy link
Owner

Choose a reason for hiding this comment

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

I'd like not to introduce static definitions in headers. Instead we can declare this constant as extern here and define it in a .c file.

Copy link
Owner

@emersion emersion left a comment

Choose a reason for hiding this comment

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

LGTM

@emersion emersion merged commit 23d9e4a into emersion:master Oct 10, 2018
@emersion
Copy link
Owner

Thanks!

@vilhalmer vilhalmer deleted the superstyle branch October 11, 2018 23:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants