Skip to content

Commit

Permalink
config: pop a message box for problematic options
Browse files Browse the repository at this point in the history 
Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
  • Loading branch information
@yshui
yshui committed Oct 17, 2024
1 parent 1616a73 commit b8d813d
Show file tree
Hide file tree
Showing 7 changed files with 246 additions and 126 deletions.
59 changes: 32 additions & 27 deletions src/c2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ typedef struct {
struct c2_condition_node_branch *b;
struct c2_condition_node_leaf *l;
};
bool neg;
bool neg : 1;
} c2_condition_node_ptr;

struct c2_tracked_property_key {
Expand Down Expand Up @@ -150,10 +150,10 @@ struct c2_condition_node_leaf {
C2_L_MPCRE,
} match : 3;
bool match_ignorecase : 1;
bool target_on_client : 1;
char *tgt;
unsigned int target_id;
xcb_atom_t tgtatom;
bool target_on_client;
int index;
// TODO(yshui) translate some of the pre-defined targets to
// generic window properties. e.g. `name = "xterm"`
Expand Down Expand Up @@ -324,8 +324,8 @@ static inline int c2h_b_opcmp(c2_b_op_t op1, c2_b_op_t op2) {
return c2h_b_opp(op1) - c2h_b_opp(op2);
}

static int
c2_parse_group(const char *pattern, int offset, c2_condition_node_ptr *presult, int level);
static int c2_parse_group(const char *pattern, int offset, c2_condition_node_ptr *presult,
int level, bool *deprecated);
static int c2_parse_target(const char *pattern, int offset, c2_condition_node_ptr *presult);
static int c2_parse_op(const char *pattern, int offset, c2_condition_node_ptr *presult);
static int c2_parse_pattern(const char *pattern, int offset, c2_condition_node_ptr *presult);
Expand All @@ -349,7 +349,8 @@ static inline void c2_freep(c2_condition_node_ptr *pp) {
/**
* Parse a condition string.
*/
struct c2_condition *c2_parse(struct list_node *list, const char *pattern, void *data) {
struct c2_condition *
c2_parse(struct list_node *list, const char *pattern, void *data, bool *deprecated) {
if (!pattern) {
return NULL;
}
Expand All @@ -361,7 +362,7 @@ struct c2_condition *c2_parse(struct list_node *list, const char *pattern, void
if (strlen(pattern) >= 2 && ':' == pattern[1]) {
offset = c2_parse_legacy(pattern, 0, &result);
} else {
offset = c2_parse_group(pattern, 0, &result, 0);
offset = c2_parse_group(pattern, 0, &result, 0, deprecated);
}

if (offset < 0) {
Expand Down Expand Up @@ -397,13 +398,13 @@ struct c2_condition *c2_parse(struct list_node *list, const char *pattern, void
c2_condition *
c2_parse_with_prefix(struct list_node *list, const char *pattern,
void *(*parse_prefix)(const char *input, const char **end, void *),
void (*free_value)(void *), void *user_data) {
void (*free_value)(void *), void *user_data, bool *deprecated) {
char *pattern_start = NULL;
void *val = parse_prefix(pattern, (const char **)&pattern_start, user_data);
if (pattern_start == NULL) {
return NULL;
}
auto ret = c2_parse(list, pattern_start, val);
auto ret = c2_parse(list, pattern_start, val, deprecated);
if (!ret && free_value) {
free_value(val);
}
Expand All @@ -412,7 +413,8 @@ c2_parse_with_prefix(struct list_node *list, const char *pattern,

TEST_CASE(c2_parse) {
char str[1024];
struct c2_condition *cond = c2_parse(NULL, "name = \"xterm\"", NULL);
bool deprecated = false;
struct c2_condition *cond = c2_parse(NULL, "name = \"xterm\"", NULL, &deprecated);
struct atom *atoms = init_mock_atoms();
struct c2_state *state = c2_state_new(atoms);
TEST_NOTEQUAL(cond, NULL);
Expand All @@ -439,16 +441,16 @@ TEST_CASE(c2_parse) {
destroy_atoms(atoms);
c2_free_condition(cond, NULL);

cond = c2_parse(NULL, "argb", NULL);
cond = c2_parse(NULL, "argb", NULL, &deprecated);
TEST_NOTEQUAL(cond, NULL);
TEST_NOTEQUAL(cond->root.type, C2_NODE_TYPE_BRANCH);
TEST_EQUAL(cond->root.l->ptntype, C2_L_PTINT);
c2_free_condition(cond, NULL);

cond = c2_parse(NULL, "argb = 'b'", NULL);
cond = c2_parse(NULL, "argb = 'b'", NULL, &deprecated);
TEST_EQUAL(cond, NULL);

cond = c2_parse(NULL, "_GTK_FRAME_EXTENTS@:c", NULL);
cond = c2_parse(NULL, "_GTK_FRAME_EXTENTS@:c", NULL, &deprecated);
TEST_NOTEQUAL(cond, NULL);
TEST_NOTEQUAL(cond->root.type, C2_NODE_TYPE_BRANCH);
TEST_NOTEQUAL(cond->root.l, NULL);
Expand All @@ -474,14 +476,14 @@ TEST_CASE(c2_parse) {
TEST_STREQUAL3(str, "_GTK_FRAME_EXTENTS@[0]", len);
c2_free_condition(cond, NULL);

cond = c2_parse(
NULL, "!(name != \"xterm\" && class_g *= \"XTerm\") || !name != \"yterm\"", NULL);
cond = c2_parse(NULL, "!(name != \"xterm\" && class_g *= \"XTerm\") || !name != \"yterm\"",
NULL, &deprecated);
TEST_NOTEQUAL(cond, NULL);
TEST_STREQUAL(c2_condition_to_str(cond), "(!(name != \"xterm\" && class_g *= "
"\"XTerm\") || name = \"yterm\")");
c2_free_condition(cond, NULL);

cond = c2_parse(NULL, "name = \"xterm\" && class_g *= \"XTerm\"", NULL);
cond = c2_parse(NULL, "name = \"xterm\" && class_g *= \"XTerm\"", NULL, &deprecated);
TEST_NOTEQUAL(cond, NULL);
TEST_EQUAL(cond->root.type, C2_NODE_TYPE_BRANCH);
TEST_NOTEQUAL(cond->root.b, NULL);
Expand Down Expand Up @@ -511,7 +513,8 @@ TEST_CASE(c2_parse) {
c2_state_free(state);
destroy_atoms(atoms);

cond = c2_parse(NULL, "_NET_WM_STATE[1]:32a *='_NET_WM_STATE_HIDDEN'", NULL);
cond = c2_parse(NULL, "_NET_WM_STATE[1]:32a *='_NET_WM_STATE_HIDDEN'", NULL,
&deprecated);
TEST_EQUAL(cond->root.l->index, 1);
TEST_STREQUAL(cond->root.l->tgt, "_NET_WM_STATE");
TEST_STREQUAL(cond->root.l->ptnstr, "_NET_WM_STATE_HIDDEN");
Expand All @@ -520,39 +523,39 @@ TEST_CASE(c2_parse) {
TEST_STREQUAL3(str, "_NET_WM_STATE[1] *= \"_NET_WM_STATE_HIDDEN\"", len);
c2_free_condition(cond, NULL);

cond = c2_parse(NULL, "_NET_WM_STATE[*]:32a*='_NET_WM_STATE_HIDDEN'", NULL);
cond = c2_parse(NULL, "_NET_WM_STATE[*]:32a*='_NET_WM_STATE_HIDDEN'", NULL, &deprecated);
TEST_EQUAL(cond->root.l->index, -1);

len = c2_condition_node_to_str(cond->root, str, sizeof(str));
TEST_STREQUAL3(str, "_NET_WM_STATE[*] *= \"_NET_WM_STATE_HIDDEN\"", len);
c2_free_condition(cond, NULL);

cond = c2_parse(NULL, "!class_i:0s", NULL);
cond = c2_parse(NULL, "!class_i:0s", NULL, &deprecated);
TEST_NOTEQUAL(cond, NULL);
len = c2_condition_node_to_str(cond->root, str, sizeof(str));
TEST_STREQUAL3(str, "!class_i", len);
c2_free_condition(cond, NULL);

cond = c2_parse(NULL, "_NET_WM_STATE = '_NET_WM_STATE_HIDDEN'", NULL);
cond = c2_parse(NULL, "_NET_WM_STATE = '_NET_WM_STATE_HIDDEN'", NULL, &deprecated);
TEST_NOTEQUAL(cond, NULL);
c2_free_condition(cond, NULL);

cond = c2_parse(NULL, "1A:\n1111111111111ar1", NULL);
cond = c2_parse(NULL, "1A:\n1111111111111ar1", NULL, &deprecated);
TEST_EQUAL(cond, NULL);

cond = c2_parse(NULL, "N [4444444444444: \n", NULL);
cond = c2_parse(NULL, "N [4444444444444: \n", NULL, &deprecated);
TEST_EQUAL(cond, NULL);

cond = c2_parse(NULL, " x:a=\"b\377\\xCCCCC", NULL);
cond = c2_parse(NULL, " x:a=\"b\377\\xCCCCC", NULL, &deprecated);
TEST_EQUAL(cond, NULL);

cond = c2_parse(NULL, "!!!!!!!((((((!(((((,", NULL);
cond = c2_parse(NULL, "!!!!!!!((((((!(((((,", NULL, &deprecated);
TEST_EQUAL(cond, NULL);

const char *rule = "(((role = \"\\\\tg^\\n\\n[\\t\" && role ~?= \"\") && "
"role ~?= \"\\n\\n\\n\\b\\n^\\n*0bon\") && role ~?= "
"\"\\n\\n\\x8a\\b\\n^\\n*0\\n[\\n[\\n\\n\\b\\n\")";
cond = c2_parse(NULL, rule, NULL);
cond = c2_parse(NULL, rule, NULL, &deprecated);
TEST_NOTEQUAL(cond, NULL);
len = c2_condition_node_to_str(cond->root, str, sizeof(str));
TEST_STREQUAL3(str, rule, len);
Expand Down Expand Up @@ -580,8 +583,8 @@ TEST_CASE(c2_parse) {
*
* @return offset of next character in string
*/
static int
c2_parse_group(const char *pattern, int offset, c2_condition_node_ptr *presult, int level) {
static int c2_parse_group(const char *pattern, int offset, c2_condition_node_ptr *presult,
int level, bool *deprecated) {
if (!pattern) {
return -1;
}
Expand Down Expand Up @@ -687,7 +690,8 @@ c2_parse_group(const char *pattern, int offset, c2_condition_node_ptr *presult,

// It's a subgroup if it starts with '('
if ('(' == pattern[offset]) {
if ((offset = c2_parse_group(pattern, offset + 1, pele, level + 1)) < 0) {
if ((offset = c2_parse_group(pattern, offset + 1, pele, level + 1,
deprecated)) < 0) {
goto fail;
}
} else {
Expand Down Expand Up @@ -716,6 +720,7 @@ c2_parse_group(const char *pattern, int offset, c2_condition_node_ptr *presult,
"always fail. Offending pattern is "
"\"%s\"",
pele->l->tgt, pattern);
*deprecated = true;

Check warning on line 723 in src/c2.c

View check run for this annotation

Codecov / codecov/patch

src/c2.c#L723 

Added line #L723 was not covered by tests
}
if (pele->l->op == C2_L_OEXISTS) {
pele->l->ptntype = predef_type;
Expand Down
5 changes: 3 additions & 2 deletions src/c2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ struct win;
struct list_node;

typedef void (*c2_userdata_free)(void *);
struct c2_condition *c2_parse(struct list_node *list, const char *pattern, void *data);
struct c2_condition *
c2_parse(struct list_node *list, const char *pattern, void *data, bool *deprecated);

/// Parse a condition that has a prefix. The prefix is parsed by `parse_prefix`. If
/// `free_value` is not NULL, it will be called to free the value returned by
/// `parse_prefix` when error occurs.
c2_condition *
c2_parse_with_prefix(struct list_node *list, const char *pattern,
void *(*parse_prefix)(const char *input, const char **end, void *),
void (*free_value)(void *), void *user_data);
void (*free_value)(void *), void *user_data, bool *deprecated);

void c2_free_condition(c2_condition *lp, c2_userdata_free f);

Expand Down
33 changes: 32 additions & 1 deletion src/config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <stdbool.h>
#include <stddef.h>
#include <string.h>
#include <uthash.h>
#include <xcb/render.h> // for xcb_render_fixed_t, XXX
#include <xcb/xcb.h>
#include <xcb/xfixes.h>
Expand Down Expand Up @@ -217,8 +218,16 @@ struct window_options {
struct win_script animations[ANIMATION_TRIGGER_COUNT];
};

struct option_name {
UT_hash_handle hh;
const char *name;
};

/// Structure representing all options.
typedef struct options {
// === Deprecation ===
struct option_name *problematic_options;

// === Config ===
/// Path to the config file
char *config_file_path;
Expand Down Expand Up @@ -429,6 +438,26 @@ typedef struct options {
extern const char *const BACKEND_STRS[NUM_BKEND + 1];

bool load_plugin(const char *name, const char *include_dir);
static inline void record_problematic_option(struct options *opt, const char *name) {
struct option_name *record = calloc(1, sizeof(*record));
record->name = name;
HASH_ADD_STR(opt->problematic_options, name, record);

Check warning on line 444 in src/config.h

View check run for this annotation

Codecov / codecov/patch

src/config.h#L441-L444 

Added lines #L441 - L444 were not covered by tests
}

static inline void
report_deprecated_option(struct options *opt, const char *name, bool error) {
struct option_name *record = NULL;
HASH_FIND_STR(opt->problematic_options, name, record);
if (record != NULL) {
return;

Check warning on line 452 in src/config.h

View check run for this annotation

Codecov / codecov/patch

src/config.h#L448-L452 

Added lines #L448 - L452 were not covered by tests
}
enum log_level level = error ? LOG_LEVEL_ERROR : LOG_LEVEL_WARN;
LOG_(level,

Check warning on line 455 in src/config.h

View check run for this annotation

Codecov / codecov/patch

src/config.h#L454-L455 

Added lines #L454 - L455 were not covered by tests
"Option \"%s\" is deprecated, please remove it from your config file and/or "
"command line options.",
name);
record_problematic_option(opt, name);

Check warning on line 459 in src/config.h

View check run for this annotation

Codecov / codecov/patch

src/config.h#L459 

Added line #L459 was not covered by tests
}

bool must_use parse_long(const char *, long *);
bool must_use parse_int(const char *, int *);
Expand Down Expand Up @@ -485,10 +514,12 @@ static inline bool parse_vsync(const char *str) {
/// Generate animation script for legacy fading options
void generate_fading_config(struct options *opt);

static inline void log_warn_both_style_of_rules(const char *option_name) {
static inline void log_warn_both_style_of_rules(struct options *opt, const char *option_name) {

Check warning on line 517 in src/config.h

View check run for this annotation

Codecov / codecov/patch

src/config.h#L517 

Added line #L517 was not covered by tests
log_warn("Option \"%s\" is set along with \"rules\". \"rules\" will take "
"precedence, and \"%s\" will have no effect.",
option_name, option_name);
opt->has_both_style_of_rules = true;
record_problematic_option(opt, option_name);

Check warning on line 522 in src/config.h

View check run for this annotation

Codecov / codecov/patch

src/config.h#L521-L522 

Added lines #L521 - L522 were not covered by tests
}
enum animation_trigger parse_animation_trigger(const char *trigger);

Expand Down
Loading

0 comments on commit b8d813d

Please sign in to comment.