From bcc61c14d48c6329cf786d9b5e10af50637c43ff Mon Sep 17 00:00:00 2001 From: Patrick Gaskin Date: Fri, 24 Apr 2020 20:24:34 -0400 Subject: [PATCH] Refactored actions * New return type which allows showing messages or toasts. * Unified action header. * More macros to make stuff consistent. * A few bugfixes. --- .gitignore | 1 + Makefile | 2 +- res/doc | 6 ++-- src/action.c | 37 ++++++++++++++++++++++++ src/action.h | 52 ++++++++++++++++++++++++++++++++++ src/action_c.c | 28 ++++++++++++------ src/action_c.h | 14 --------- src/action_cc.cc | 74 +++++++++++++----------------------------------- src/action_cc.h | 19 ------------- src/config.c | 16 ++++------- src/init.c | 5 ++-- src/menu.cc | 39 +++++++++++++++++++++++-- src/menu.h | 4 ++- 13 files changed, 181 insertions(+), 116 deletions(-) create mode 100644 src/action.c create mode 100644 src/action.h delete mode 100644 src/action_c.h delete mode 100644 src/action_cc.h diff --git a/.gitignore b/.gitignore index 8e5017a..1d21400 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ /src/failsafe.o /src/init.o /src/config.o +/src/action.o /src/dlhook.o /src/menu.o diff --git a/Makefile b/Makefile index c11897b..25b1f99 100644 --- a/Makefile +++ b/Makefile @@ -71,7 +71,7 @@ override GENERATED += KoboRoot.tgz src/libnm.so: override CFLAGS += $(PTHREAD_CFLAGS) -fPIC src/libnm.so: override CXXFLAGS += $(PTHREAD_CFLAGS) $(QT5CORE_CFLAGS) $(QT5WIDGETS_CFLAGS) -fPIC src/libnm.so: override LDFLAGS += $(PTHREAD_LIBS) $(QT5CORE_LIBS) $(QT5WIDGETS_LIBS) -ldl -Wl,-soname,libnm.so -src/libnm.so: src/qtplugin.o src/init.o src/config.o src/dlhook.o src/failsafe.o src/menu.o src/action_c.o src/action_cc.o +src/libnm.so: src/qtplugin.o src/init.o src/config.o src/dlhook.o src/failsafe.o src/menu.o src/action.o src/action_c.o src/action_cc.o override LIBRARIES += src/libnm.so override MOCS += src/qtplugin.moc diff --git a/res/doc b/res/doc index 6128778..30e0abc 100644 --- a/res/doc +++ b/res/doc @@ -21,6 +21,8 @@ # the type of action to run, one of: # dbg_syslog - writes a message to syslog (for testing) # dbg_error - always returns an error (for testing) +# dbg_msg - shows a message (for testing) +# dbg_toast - shows a toast (for testing) # kfmon - triggers a kfmon action (TODO) # nickel_setting - toggles a boolean setting # nickel_extras - opens one of the beta features @@ -33,9 +35,9 @@ # kfmon - TODO # nickel_setting - one of: # invert - toggles FeatureSettings.InvertScreen (all versions) -# screenshots - toggles FeatureSettings.Screenshots (all versions) (TODO) +# screenshots - toggles FeatureSettings.Screenshots (all versions) # nickel_extras - the mimetype of the plugin, or one of: -# web_browser (TODO) +# web_browser # unblock_it # sketch_pad # solitaire diff --git a/src/action.c b/src/action.c new file mode 100644 index 0000000..0762e8d --- /dev/null +++ b/src/action.c @@ -0,0 +1,37 @@ +#define _GNU_SOURCE // vasprintf +#include +#include +#include +#include + +#include "action.h" + +nm_action_result_t *nm_action_result_silent() { + nm_action_result_t *res = calloc(1, sizeof(nm_action_result_t)); + res->type = NM_ACTION_RESULT_TYPE_SILENT; + return res; +} + +#define _nm_action_result_fmt(_fn, _typ) \ + nm_action_result_t *nm_action_result_##_fn(const char *fmt, ...) { \ + nm_action_result_t *res = calloc(1, sizeof(nm_action_result_t)); \ + res->type = _typ; \ + va_list v; \ + va_start(v, fmt); \ + if (vasprintf(&res->msg, fmt, v) == -1) \ + res->msg = strdup("error"); \ + va_end(v); \ + return res; \ + } + +_nm_action_result_fmt(msg, NM_ACTION_RESULT_TYPE_MSG); +_nm_action_result_fmt(toast, NM_ACTION_RESULT_TYPE_TOAST); + +void nm_action_result_free(nm_action_result_t *res) { + if (!res) + return; + + free(res); + if (res->msg) + free(res->msg); +} diff --git a/src/action.h b/src/action.h new file mode 100644 index 0000000..aad62ee --- /dev/null +++ b/src/action.h @@ -0,0 +1,52 @@ +#ifndef NM_ACTION_H +#define NM_ACTION_H +#ifdef __cplusplus +extern "C" { +#endif + +typedef enum { + NM_ACTION_RESULT_TYPE_SILENT = 0, + NM_ACTION_RESULT_TYPE_MSG = 1, + NM_ACTION_RESULT_TYPE_TOAST = 2, +} nm_action_result_type_t; + +typedef struct { + nm_action_result_type_t type; + char *msg; +} nm_action_result_t; + +typedef nm_action_result_t *(*nm_action_fn_t)(const char *arg, char **err); + +nm_action_result_t *nm_action_result_silent(); +nm_action_result_t *nm_action_result_msg(const char *fmt, ...) __attribute__((format(printf, 1, 2))); +nm_action_result_t *nm_action_result_toast(const char *fmt, ...) __attribute__((format(printf, 1, 2))); +void nm_action_result_free(nm_action_result_t *res); + +#define NM_ACTION(name) nm_action_##name + +#ifdef __cplusplus +#define NM_ACTION_(name) extern "C" nm_action_result_t *NM_ACTION(name)(const char *arg, char **err_out) +#else +#define NM_ACTION_(name) nm_action_result_t *NM_ACTION(name)(const char *arg, char **err_out) +#endif + +#define NM_ACTIONS \ + X(dbg_syslog) \ + X(dbg_error) \ + X(dbg_msg) \ + X(dbg_toast) \ + X(kfmon) \ + X(nickel_setting) \ + X(nickel_extras) \ + X(nickel_misc) \ + X(cmd_spawn) \ + X(cmd_output) + +#define X(name) NM_ACTION_(name); +NM_ACTIONS +#undef X + +#ifdef __cplusplus +} +#endif +#endif \ No newline at end of file diff --git a/src/action_c.c b/src/action_c.c index 2f859ca..2fc8adc 100644 --- a/src/action_c.c +++ b/src/action_c.c @@ -1,23 +1,35 @@ #define _GNU_SOURCE // asprintf -#include "action_c.h" +#include "action.h" #include "util.h" -int nm_action_dbgsyslog(const char *arg, char **err_out) { - #define NM_ERR_RET 1 +NM_ACTION_(dbg_syslog) { + #define NM_ERR_RET NULL NM_LOG("dbgsyslog: %s", arg); - NM_RETURN_OK(0); + NM_RETURN_OK(nm_action_result_silent()); #undef NM_ERR_RET } -int nm_action_dbgerror(const char *arg, char **err_out) { - #define NM_ERR_RET 1 +NM_ACTION_(dbg_error) { + #define NM_ERR_RET NULL NM_RETURN_ERR("%s", arg); #undef NM_ERR_RET } -int nm_action_kfmon(const char *arg, char **err_out) { - #define NM_ERR_RET 1 +NM_ACTION_(dbg_msg) { + #define NM_ERR_RET NULL + NM_RETURN_OK(nm_action_result_msg("%s", arg)); + #undef NM_ERR_RET +} + +NM_ACTION_(dbg_toast) { + #define NM_ERR_RET NULL + NM_RETURN_OK(nm_action_result_toast("%s", arg)); + #undef NM_ERR_RET +} + +NM_ACTION_(kfmon) { + #define NM_ERR_RET NULL NM_RETURN_ERR("not implemented yet (arg=%s)", arg); // TODO #undef NM_ERR_RET } diff --git a/src/action_c.h b/src/action_c.h deleted file mode 100644 index 375a43e..0000000 --- a/src/action_c.h +++ /dev/null @@ -1,14 +0,0 @@ -#ifndef NM_ACTION_C_H -#define NM_ACTION_C_H -#ifdef __cplusplus -extern "C" { -#endif - -int nm_action_dbgsyslog(const char *arg, char **err_out); -int nm_action_dbgerror(const char *arg, char **err_out); -int nm_action_kfmon(const char *arg, char **err_out); - -#ifdef __cplusplus -} -#endif -#endif diff --git a/src/action_cc.cc b/src/action_cc.cc index fbbd79e..34c35ea 100644 --- a/src/action_cc.cc +++ b/src/action_cc.cc @@ -17,7 +17,7 @@ #include #include -#include "action_cc.h" +#include "action.h" #include "util.h" typedef void Device; @@ -25,33 +25,9 @@ typedef void Settings; typedef void PlugWorkflowManager; typedef void BrowserWorkflowManager; typedef void N3SettingsExtrasController; -typedef void MainWindowController; -static void toast(QString const& primaryText, QString const& secondaryText, int msecs) { - MainWindowController *(*MainWindowController_sharedInstance)(); - reinterpret_cast(MainWindowController_sharedInstance) = dlsym(RTLD_DEFAULT, "_ZN20MainWindowController14sharedInstanceEv"); - if (!MainWindowController_sharedInstance) { - NM_LOG("toast: could not dlsym MainWindowController::sharedInstance"); - return; - } - - void (*MainWindowController_toast)(MainWindowController*, QString const&, QString const&, int); - reinterpret_cast(MainWindowController_toast) = dlsym(RTLD_DEFAULT, "_ZN20MainWindowController5toastERK7QStringS2_i"); - if (!MainWindowController_toast) { - NM_LOG("toast: could not dlsym MainWindowController::toast"); - return; - } - - MainWindowController *mwc = MainWindowController_sharedInstance(); - if (!mwc) { - NM_LOG("toast: could not get shared main window controller pointer"); - return; - } - MainWindowController_toast(mwc, primaryText, secondaryText, msecs); -} - -extern "C" int nm_action_nickelsetting(const char *arg, char **err_out) { - #define NM_ERR_RET 1 +NM_ACTION_(nickel_setting) { + #define NM_ERR_RET nullptr Device *(*Device_getCurrentDevice)(); reinterpret_cast(Device_getCurrentDevice) = dlsym(RTLD_DEFAULT, "_ZN6Device16getCurrentDeviceEv"); @@ -137,15 +113,14 @@ extern "C" int nm_action_nickelsetting(const char *arg, char **err_out) { Settings_SettingsD(settings); - if (strcmp(arg, "invert")) // invert is obvious - toast(arg, v ? QStringLiteral("Disabled") : QStringLiteral("Enabled"), 1500); - - NM_RETURN_OK(0); + NM_RETURN_OK(strcmp(arg, "invert") // invert is obvious + ? nm_action_result_toast("%s %s", v ? "disabled" : "enabled", arg) + : nm_action_result_silent()); #undef NM_ERR_RET } -extern "C" int nm_action_nickelextras(const char *arg, char **err_out) { - #define NM_ERR_RET 1 +NM_ACTION_(nickel_extras) { + #define NM_ERR_RET nullptr if (!strcmp(arg, "web_browser")) { void (*N3SettingsExtrasController_N3SettingsExtrasController)(N3SettingsExtrasController*); @@ -177,7 +152,7 @@ extern "C" int nm_action_nickelextras(const char *arg, char **err_out) { BrowserWorkflowManager *bwm = alloca(128); // as of 4.20.14622, it's actually 20 bytes, but we're going to stay on the safe side BrowserWorkflowManager_BrowserWorkflowManager(bwm, new QObject()); BrowserWorkflowManager_openBrowser(bwm, false, QUrl(), QStringLiteral("")); // if !QUrl::isValid(), it loads the homepage*/ - NM_RETURN_OK(0); + NM_RETURN_OK(nm_action_result_silent()); } const char* mimetype; @@ -194,12 +169,12 @@ extern "C" int nm_action_nickelextras(const char *arg, char **err_out) { NM_ASSERT(ExtrasPluginLoader_loadPlugin, "could not dlsym ExtrasPluginLoader::loadPlugin"); ExtrasPluginLoader_loadPlugin(mimetype); - NM_RETURN_OK(0); + NM_RETURN_OK(nm_action_result_silent()); #undef NM_ERR_RET } -extern "C" int nm_action_nickelmisc(const char *arg, char **err_out) { - #define NM_ERR_RET 1 +NM_ACTION_(nickel_misc) { + #define NM_ERR_RET nullptr if (!strcmp(arg, "rescan_books")) { PlugWorkflowManager *(*PlugWorkflowManager_sharedInstance)(); reinterpret_cast(PlugWorkflowManager_sharedInstance) = dlsym(RTLD_DEFAULT, "_ZN19PlugWorkflowManager14sharedInstanceEv"); @@ -245,17 +220,12 @@ extern "C" int nm_action_nickelmisc(const char *arg, char **err_out) { } else { NM_RETURN_ERR("unknown action '%s'", arg); } - NM_RETURN_OK(0); + NM_RETURN_OK(nm_action_result_silent()); #undef NM_ERR_RET } -// TODO: stop abusing err_out to display messages, maybe add a msg_out arg -// for that kind of thing instead (I've marked those spots by returning 2 for -// now). - -extern "C" int nm_action_cmdspawn(const char *arg, char **err_out) { - #define NM_ERR_RET 1 - +NM_ACTION_(cmd_spawn) { + #define NM_ERR_RET nullptr QProcess proc; uint64_t pid; bool ok = proc.startDetached( @@ -268,16 +238,12 @@ extern "C" int nm_action_cmdspawn(const char *arg, char **err_out) { (qint64*)(&pid) ); NM_ASSERT(ok, "could not start process"); - - if (*err_out) - asprintf(err_out, "Successfully started process with PID %lu.", (unsigned long)(pid)); - return 2; - + NM_RETURN_OK(nm_action_result_toast("Successfully started process with PID %lu.", (unsigned long)(pid))); #undef NM_ERR_RET } -extern "C" int nm_action_cmdoutput(const char *arg, char **err_out) { - #define NM_ERR_RET 1 +NM_ACTION_(cmd_output) { + #define NM_ERR_RET nullptr char *tmp = strdup(arg); @@ -320,9 +286,7 @@ extern "C" int nm_action_cmdoutput(const char *arg, char **err_out) { free(tmp); - if (err_out) - *err_out = strdup(qPrintable(out)); - return 2; + NM_RETURN_OK(nm_action_result_msg("%s", qPrintable(out))); #undef NM_ERR_RET } diff --git a/src/action_cc.h b/src/action_cc.h deleted file mode 100644 index 3d7345a..0000000 --- a/src/action_cc.h +++ /dev/null @@ -1,19 +0,0 @@ -#ifndef NM_ACTION_CC_H -#define NM_ACTION_CC_H -#ifdef __cplusplus -extern "C" { -#endif - -// This file contains actions which need access to Qt classes, but is still -// mostly C code. - -int nm_action_nickelsetting(const char *arg, char **err_out); -int nm_action_nickelextras(const char *arg, char **err_out); -int nm_action_nickelmisc(const char *arg, char **err_out); -int nm_action_cmdspawn(const char *arg, char **err_out); -int nm_action_cmdoutput(const char *arg, char **err_out); - -#ifdef __cplusplus -} -#endif -#endif diff --git a/src/config.c b/src/config.c index b87c15e..ccb49bf 100644 --- a/src/config.c +++ b/src/config.c @@ -10,8 +10,7 @@ #include #include -#include "action_c.h" -#include "action_cc.h" +#include "action.h" #include "config.h" #include "menu.h" #include "util.h" @@ -130,14 +129,9 @@ nm_config_t *nm_config_parse(char **err_out) { // type: menu_item - field 4: action char *c_act = strtrim(strsep(&cur, ":")); if (!c_act) RETERR("file %s: line %d: field 4: expected action, got end of line", fn, line_n); - else if (!strcmp(c_act, "dbg_syslog")) it->act = nm_action_dbgsyslog; - else if (!strcmp(c_act, "dbg_error")) it->act = nm_action_dbgerror; - else if (!strcmp(c_act, "kfmon")) it->act = nm_action_kfmon; - else if (!strcmp(c_act, "nickel_setting")) it->act = nm_action_nickelsetting; - else if (!strcmp(c_act, "nickel_extras")) it->act = nm_action_nickelextras; - else if (!strcmp(c_act, "nickel_misc")) it->act = nm_action_nickelmisc; - else if (!strcmp(c_act, "cmd_spawn")) it->act = nm_action_cmdspawn; - else if (!strcmp(c_act, "cmd_output")) it->act = nm_action_cmdoutput; + #define X(name) else if (!strcmp(c_act, #name)) it->act = NM_ACTION(name); + NM_ACTIONS + #undef X else RETERR("file %s: line %d: field 4: unknown action '%s'", fn, line_n, c_act); // type: menu_item - field 5: argument @@ -165,7 +159,7 @@ nm_config_t *nm_config_parse(char **err_out) { it->loc = NM_MENU_LOCATION_MAIN_MENU; it->lbl = strdup("NickelMenu"); it->arg = strdup("See KOBOeReader/.add/nm/doc for instructions on how to customize this menu."); - it->act = nm_action_dbgerror; + it->act = NM_ACTION(dbg_toast); nm_config_push_menu_item(&cfg, it); } diff --git a/src/init.c b/src/init.c index 13fee32..f74164b 100644 --- a/src/init.c +++ b/src/init.c @@ -7,8 +7,7 @@ #include #include -#include "action_c.h" -#include "action_cc.h" +#include "action.h" #include "config.h" #include "failsafe.h" #include "menu.h" @@ -52,7 +51,7 @@ __attribute__((constructor)) void nm_init() { items[0]->loc = NM_MENU_LOCATION_MAIN_MENU; items[0]->lbl = strdup("Config Error"); items[0]->arg = strdup(err); - items[0]->act = nm_action_dbgerror; + items[0]->act = NM_ACTION(dbg_msg); free(err); } else if (!(items = nm_config_get_menu(cfg, &items_n))) { diff --git a/src/menu.cc b/src/menu.cc index 1eb00f6..6121227 100644 --- a/src/menu.cc +++ b/src/menu.cc @@ -7,11 +7,13 @@ #include #include +#include "action.h" #include "dlhook.h" #include "menu.h" #include "util.h" typedef QWidget MenuTextItem; // it's actually a subclass, but we don't need it's functionality directly, so we'll stay on the safe side +typedef void MainWindowController; // AbstractNickelMenuController::createMenuTextItem creates a menu item in the // given QMenu with the specified label. The first bool parameter adds space to @@ -33,6 +35,13 @@ static QAction* (*AbstractNickelMenuController_createAction)(void*, QMenu*, QWid // a signal handler). static void (*ConfirmationDialogFactory_showOKDialog)(QString const&, QString const&); +MainWindowController *(*MainWindowController_sharedInstance)(); + +// MainWindowController::toast shows a message (primary and secondary text) as +// an overlay for a number of milliseconds. It should also be called from the +// GUI thread. +void (*MainWindowController_toast)(MainWindowController*, QString const&, QString const&, int); + static nm_menu_item_t **_items; static size_t _items_n; @@ -41,10 +50,14 @@ extern "C" int nm_menu_hook(void *libnickel, nm_menu_item_t **items, size_t item reinterpret_cast(AbstractNickelMenuController_createMenuTextItem) = dlsym(libnickel, "_ZN28AbstractNickelMenuController18createMenuTextItemEP5QMenuRK7QStringbbS4_"); reinterpret_cast(AbstractNickelMenuController_createAction) = dlsym(libnickel, "_ZN22AbstractMenuController12createActionEP5QMenuP7QWidgetbbb"); reinterpret_cast(ConfirmationDialogFactory_showOKDialog) = dlsym(libnickel, "_ZN25ConfirmationDialogFactory12showOKDialogERK7QStringS2_"); + reinterpret_cast(MainWindowController_sharedInstance) = dlsym(libnickel, "_ZN20MainWindowController14sharedInstanceEv"); + reinterpret_cast(MainWindowController_toast) = dlsym(libnickel, "_ZN20MainWindowController5toastERK7QStringS2_i"); NM_ASSERT(AbstractNickelMenuController_createMenuTextItem, "unsupported firmware: could not find AbstractNickelMenuController::createMenuTextItem(void* _this, QMenu*, QString, bool, bool, QString const&)"); NM_ASSERT(AbstractNickelMenuController_createAction, "unsupported firmware: could not find AbstractNickelMenuController::createAction(void* _this, QMenu*, QWidget*, bool, bool, bool)"); - NM_ASSERT(AbstractNickelMenuController_createAction, "unsupported firmware: could not find ConfirmationDialogFactory::showOKDialog(QString const&, QString const&)"); + NM_ASSERT(ConfirmationDialogFactory_showOKDialog, "unsupported firmware: could not find ConfirmationDialogFactory::showOKDialog(String const&, QString const&)"); + NM_ASSERT(MainWindowController_sharedInstance, "unsupported firmware: could not find MainWindowController::sharedInstance()"); + NM_ASSERT(MainWindowController_toast, "unsupported firmware: could not find MainWindowController_toast(QString const&, QString const&, int)"); void* nmh = dlsym(RTLD_DEFAULT, "_nm_menu_hook"); NM_ASSERT(nmh, "internal error: could not dlsym _nm_menu_hook"); @@ -89,11 +102,33 @@ extern "C" MenuTextItem* _nm_menu_hook(void* _this, QMenu* menu, QString const& QObject::connect(action, &QAction::triggered, std::function([it](bool){ NM_LOG("Item '%s' pressed...", it->lbl); char *err; - if (it->act(it->arg, &err) && err) { + nm_action_result_t *res = it->act(it->arg, &err); + if (err) { NM_LOG("Got error: '%s', displaying...", err); ConfirmationDialogFactory_showOKDialog(QString::fromUtf8(it->lbl), QString::fromUtf8(err)); free(err); return; + } else if (res) { + NM_LOG("Got result: type=%d msg='%s', handling...", res->type, res->msg); + MainWindowController *mwc; + switch (res->type) { + case NM_ACTION_RESULT_TYPE_SILENT: + break; + case NM_ACTION_RESULT_TYPE_MSG: + ConfirmationDialogFactory_showOKDialog(QString::fromUtf8(it->lbl), QLatin1String(res->msg)); + break; + case NM_ACTION_RESULT_TYPE_TOAST: + mwc = MainWindowController_sharedInstance(); + if (!mwc) { + NM_LOG("toast: could not get shared main window controller pointer"); + break; + } + MainWindowController_toast(mwc, QLatin1String(res->msg), QStringLiteral(""), 1500); + break; + } + nm_action_result_free(res); + } else { + NM_LOG("warning: you should have returned a result with type silent, not null, upon success"); } NM_LOG("Success!"); })); diff --git a/src/menu.h b/src/menu.h index 61c0569..95ceae9 100644 --- a/src/menu.h +++ b/src/menu.h @@ -6,6 +6,8 @@ extern "C" { #include +#include "action.h" + typedef enum { NM_MENU_LOCATION_MAIN_MENU = 1, NM_MENU_LOCATION_READER_MENU = 2, @@ -15,7 +17,7 @@ typedef struct { nm_menu_location_t loc; char *lbl; char *arg; - int (*act)(const char *arg, char **out_err); // can block, must return 0 on success, nonzero with out_err set to the malloc'd error message on error + nm_action_fn_t act; // can block, must return 0 on success, nonzero with out_err set to the malloc'd error message on error } nm_menu_item_t; // nm_menu_hook hooks a dlopen'd libnickel handle to add the specified menus,