Instead of returning a bool from `nm_global_config_update` indicating whether
the menu items were modified, return an int indicating the current config
revision which can then be compared against one stored as a property of the
menu.
This issue was found by @jackiew1. See #56 for more details.
This fixes an issue in ee0eb7ccf7 (#47).
This commit is contained in:
17
src/init.c
17
src/init.c
@@ -60,13 +60,13 @@ __attribute__((constructor)) void nm_init() {
|
|||||||
|
|
||||||
NM_LOG("init: updating config");
|
NM_LOG("init: updating config");
|
||||||
|
|
||||||
bool upd = nm_global_config_update(&err);
|
int rev = nm_global_config_update(&err);
|
||||||
if (err) {
|
if (err) {
|
||||||
NM_LOG("init: error parsing config, will show a menu item with the error: %s", err);
|
NM_LOG("init: error parsing config, will show a menu item with the error: %s", err);
|
||||||
}
|
}
|
||||||
|
|
||||||
size_t ntmp = SIZE_MAX;
|
size_t ntmp = SIZE_MAX;
|
||||||
if (!upd) {
|
if (rev == -1) {
|
||||||
NM_LOG("init: no config file changes detected for initial config update (it should always return an error or update), stopping (this is a bug; err should have been returned instead)");
|
NM_LOG("init: no config file changes detected for initial config update (it should always return an error or update), stopping (this is a bug; err should have been returned instead)");
|
||||||
return;
|
return;
|
||||||
} else if (!nm_global_config_items(&ntmp)) {
|
} else if (!nm_global_config_items(&ntmp)) {
|
||||||
@@ -106,7 +106,8 @@ stop:
|
|||||||
static nm_config_file_t *nm_global_menu_config_files = NULL; // updated in-place by nm_global_config_update
|
static nm_config_file_t *nm_global_menu_config_files = NULL; // updated in-place by nm_global_config_update
|
||||||
static nm_config_t *nm_global_menu_config = NULL; // updated by nm_global_config_update, replaced by nm_global_config_replace, NULL on error
|
static nm_config_t *nm_global_menu_config = NULL; // updated by nm_global_config_update, replaced by nm_global_config_replace, NULL on error
|
||||||
static nm_menu_item_t **nm_global_menu_config_items = NULL; // updated by nm_global_config_replace to an error message or the items from nm_global_menu_config
|
static nm_menu_item_t **nm_global_menu_config_items = NULL; // updated by nm_global_config_replace to an error message or the items from nm_global_menu_config
|
||||||
static size_t nm_global_menu_config_n = 0; // ^
|
static size_t nm_global_menu_config_n = 0; // ^
|
||||||
|
static int nm_global_menu_config_rev = -1; // incremented by nm_global_config_update whenever the config items change for any reason
|
||||||
|
|
||||||
nm_menu_item_t **nm_global_config_items(size_t *n_out) {
|
nm_menu_item_t **nm_global_config_items(size_t *n_out) {
|
||||||
if (n_out)
|
if (n_out)
|
||||||
@@ -158,8 +159,8 @@ static void nm_global_config_replace(nm_config_t *cfg, const char *err) {
|
|||||||
NM_LOG("could not allocate memory");
|
NM_LOG("could not allocate memory");
|
||||||
}
|
}
|
||||||
|
|
||||||
bool nm_global_config_update(char **err_out) {
|
int nm_global_config_update(char **err_out) {
|
||||||
#define NM_ERR_RET true
|
#define NM_ERR_RET nm_global_menu_config_rev
|
||||||
|
|
||||||
char *err;
|
char *err;
|
||||||
|
|
||||||
@@ -169,6 +170,7 @@ bool nm_global_config_update(char **err_out) {
|
|||||||
NM_LOG("... error: %s", err);
|
NM_LOG("... error: %s", err);
|
||||||
NM_LOG("global: freeing old config and replacing with error item");
|
NM_LOG("global: freeing old config and replacing with error item");
|
||||||
nm_global_config_replace(NULL, err);
|
nm_global_config_replace(NULL, err);
|
||||||
|
nm_global_menu_config_rev++;
|
||||||
NM_RETURN_ERR("scan for config files: %s", err);
|
NM_RETURN_ERR("scan for config files: %s", err);
|
||||||
}
|
}
|
||||||
NM_LOG("global:%s changes detected", updated ? "" : " no");
|
NM_LOG("global:%s changes detected", updated ? "" : " no");
|
||||||
@@ -180,11 +182,13 @@ bool nm_global_config_update(char **err_out) {
|
|||||||
NM_LOG("... error: %s", err);
|
NM_LOG("... error: %s", err);
|
||||||
NM_LOG("global: freeing old config and replacing with error item");
|
NM_LOG("global: freeing old config and replacing with error item");
|
||||||
nm_global_config_replace(NULL, err);
|
nm_global_config_replace(NULL, err);
|
||||||
|
nm_global_menu_config_rev++;
|
||||||
NM_RETURN_ERR("parse config files: %s", err);
|
NM_RETURN_ERR("parse config files: %s", err);
|
||||||
}
|
}
|
||||||
|
|
||||||
NM_LOG("global: config updated, freeing old config and replacing with new one");
|
NM_LOG("global: config updated, freeing old config and replacing with new one");
|
||||||
nm_global_config_replace(cfg, NULL);
|
nm_global_config_replace(cfg, NULL);
|
||||||
|
nm_global_menu_config_rev++;
|
||||||
NM_LOG("global: done swapping config");
|
NM_LOG("global: done swapping config");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -207,10 +211,11 @@ bool nm_global_config_update(char **err_out) {
|
|||||||
if (!nm_global_menu_config_items)
|
if (!nm_global_menu_config_items)
|
||||||
NM_LOG("could not allocate memory");
|
NM_LOG("could not allocate memory");
|
||||||
|
|
||||||
|
nm_global_menu_config_rev++;
|
||||||
NM_LOG("done replacing items");
|
NM_LOG("done replacing items");
|
||||||
}
|
}
|
||||||
|
|
||||||
NM_RETURN_OK(updated || g_updated);
|
NM_RETURN_OK(nm_global_menu_config_rev);
|
||||||
|
|
||||||
#undef NM_ERR_RET
|
#undef NM_ERR_RET
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,11 +7,10 @@ extern "C" {
|
|||||||
#include <stdbool.h>
|
#include <stdbool.h>
|
||||||
#include "menu.h"
|
#include "menu.h"
|
||||||
|
|
||||||
// nm_global_config_update updates and regenerates the config if needed, and
|
// nm_global_config_update updates and regenerates the config if needed. If the
|
||||||
// returns true if it needed updating. If an error occurs, true is also returned
|
// menu items changed (i.e. the old items aren't valid anymore), the revision
|
||||||
// since the menu items will be updated to a single one showing the error (see
|
// will be incremented and returned (even if there was an error).
|
||||||
// nm_global_config_items).
|
int nm_global_config_update(char **err_out);
|
||||||
bool nm_global_config_update(char **err_out);
|
|
||||||
|
|
||||||
// nm_global_config_items returns an array of pointers with the current menu
|
// nm_global_config_items returns an array of pointers with the current menu
|
||||||
// items (the pointer and the items it points to will remain valid until the
|
// items (the pointer and the items it points to will remain valid until the
|
||||||
|
|||||||
15
src/menu.cc
15
src/menu.cc
@@ -138,15 +138,17 @@ extern "C" NM_PUBLIC MenuTextItem* _nm_menu_hook(void* _this, QMenu* menu, QStri
|
|||||||
void _nm_menu_inject(void *nmc, QMenu *menu, nm_menu_location_t loc, int at) {
|
void _nm_menu_inject(void *nmc, QMenu *menu, nm_menu_location_t loc, int at) {
|
||||||
NM_LOG("inject %d @ %d", loc, at);
|
NM_LOG("inject %d @ %d", loc, at);
|
||||||
|
|
||||||
NM_LOG("checking for config updates");
|
int rev_o = menu->property("nm_config_rev").toInt();
|
||||||
bool updated = nm_global_config_update(NULL); // if there was an error it will be returned as a menu item anyways (and updated will be true)
|
|
||||||
NM_LOG("updated = %d", updated);
|
NM_LOG("checking for config updates (current revision: %d)", rev_o);
|
||||||
|
int rev_n = nm_global_config_update(NULL); // if there was an error it will be returned as a menu item anyways (and updated will be true)
|
||||||
|
NM_LOG("new revision = %d%s", rev_n, rev_n == rev_o ? "" : " (changed)");
|
||||||
|
|
||||||
NM_LOG("checking for existing items added by nm");
|
NM_LOG("checking for existing items added by nm");
|
||||||
|
|
||||||
for (auto action : menu->actions()) {
|
for (auto action : menu->actions()) {
|
||||||
if (action->property("nm_action") == true) {
|
if (action->property("nm_action") == true) {
|
||||||
if (!updated)
|
if (rev_o == rev_n)
|
||||||
return; // already added items, menu is up to date
|
return; // already added items, menu is up to date
|
||||||
menu->removeAction(action);
|
menu->removeAction(action);
|
||||||
delete action;
|
delete action;
|
||||||
@@ -183,7 +185,7 @@ void _nm_menu_inject(void *nmc, QMenu *menu, nm_menu_location_t loc, int at) {
|
|||||||
if (it->loc != loc)
|
if (it->loc != loc)
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
NM_LOG("adding items '%s'...", it->lbl);
|
NM_LOG("adding item '%s'...", it->lbl);
|
||||||
|
|
||||||
MenuTextItem* item = AbstractNickelMenuController_createMenuTextItem_orig(nmc, menu, QString::fromUtf8(it->lbl), false, false, "");
|
MenuTextItem* item = AbstractNickelMenuController_createMenuTextItem_orig(nmc, menu, QString::fromUtf8(it->lbl), false, false, "");
|
||||||
QAction* action = AbstractNickelMenuController_createAction_before(before, loc, i == items_n-1, nmc, menu, item, true, true, true);
|
QAction* action = AbstractNickelMenuController_createAction_before(before, loc, i == items_n-1, nmc, menu, item, true, true, true);
|
||||||
@@ -194,6 +196,9 @@ void _nm_menu_inject(void *nmc, QMenu *menu, nm_menu_location_t loc, int at) {
|
|||||||
NM_LOG("done");
|
NM_LOG("done");
|
||||||
}); // note: we're capturing by value, i.e. the pointer to the global variable, rather then the stack variable, so this is safe
|
}); // note: we're capturing by value, i.e. the pointer to the global variable, rather then the stack variable, so this is safe
|
||||||
}
|
}
|
||||||
|
|
||||||
|
NM_LOG("updating config revision property");
|
||||||
|
menu->setProperty("nm_config_rev", rev_n);
|
||||||
}
|
}
|
||||||
|
|
||||||
void nm_menu_item_do(nm_menu_item_t *it) {
|
void nm_menu_item_do(nm_menu_item_t *it) {
|
||||||
|
|||||||
Reference in New Issue
Block a user