From 1c963ff15a19510d8d17e043082ead0cd3dbde4c Mon Sep 17 00:00:00 2001 From: Arkadiy Illarionov Date: Sat, 16 Mar 2024 16:12:28 +0300 Subject: [PATCH] Enable and fix clang-tidy readability-simplify-boolean-expr Replace some macros with inline functions. --- .clang-tidy | 1 - src/bar-gps.cc | 2 +- src/bar-sort.cc | 2 +- src/editors.cc | 18 +++++++++--------- src/editors.h | 4 ++-- src/history-list.cc | 2 +- src/image.cc | 2 +- src/layout-util.cc | 10 ++++++---- src/menu.cc | 2 +- src/metadata.cc | 26 ++++++++++++++++++++------ src/renderer-tiles.cc | 2 +- src/utilops.cc | 8 ++++---- 12 files changed, 47 insertions(+), 32 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index c87a7f4d..88cf9943 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -54,7 +54,6 @@ Checks: > -readability-magic-numbers, -readability-named-parameter, -readability-qualified-auto, - -readability-simplify-boolean-expr, ExtraArgs: [-Wno-unknown-warning-option, -Wno-unused-lambda-capture, -Wno-unused-but-set-variable] WarningsAsErrors: "*" diff --git a/src/bar-gps.cc b/src/bar-gps.cc index 9d39104c..08cfdf6d 100644 --- a/src/bar-gps.cc +++ b/src/bar-gps.cc @@ -477,7 +477,7 @@ static gboolean bar_pane_gps_create_markers_cb(gpointer data) longitude = metadata_read_GPS_coord(fd, "Xmp.exif.GPSLongitude", 0); compass = metadata_read_GPS_direction(fd, "Xmp.exif.GPSImgDirection", 1000); - if (!(latitude == 0 && longitude == 0)) + if (latitude != 0 || longitude != 0) { pgd->num_added++; diff --git a/src/bar-sort.cc b/src/bar-sort.cc index 9777e88f..d2909501 100644 --- a/src/bar-sort.cc +++ b/src/bar-sort.cc @@ -211,7 +211,7 @@ static void bar_sort_undo_folder(SortData *sd, GtkWidget *button) { gchar *origin; - if (!(sd->undo_src_list && sd->undo_dest_list)) return; + if (!sd->undo_src_list || !sd->undo_dest_list) return; switch (sd->undo_action) { diff --git a/src/editors.cc b/src/editors.cc index 9daeaa37..43bba680 100644 --- a/src/editors.cc +++ b/src/editors.cc @@ -1077,7 +1077,7 @@ static EditorFlags editor_command_one(const EditorDescription *editor, GList *li ed->flags = editor->flags; ed->flags = static_cast(ed->flags | editor_command_parse(editor, list, TRUE, &command)); - ok = !EDITOR_ERRORS(ed->flags); + ok = !editor_errors(ed->flags); if (ok) { @@ -1173,7 +1173,7 @@ static EditorFlags editor_command_one(const EditorDescription *editor, GList *li g_free(command); - return static_cast(EDITOR_ERRORS(ed->flags)); + return static_cast(editor_errors(ed->flags)); } static EditorFlags editor_command_next_start(EditorData *ed) @@ -1250,7 +1250,7 @@ static EditorFlags editor_command_next_finish(EditorData *ed, gint status) switch (cont) { case EDITOR_CB_SUSPEND: - return static_cast(EDITOR_ERRORS(ed->flags)); + return static_cast(editor_errors(ed->flags)); case EDITOR_CB_SKIP: return editor_command_done(ed); } @@ -1286,7 +1286,7 @@ static EditorFlags editor_command_done(EditorData *ed) ed->count = 0; - flags = static_cast(EDITOR_ERRORS(ed->flags)); + flags = static_cast(editor_errors(ed->flags)); if (!ed->vd) editor_data_free(ed); @@ -1308,7 +1308,7 @@ static EditorFlags editor_command_start(const EditorDescription *editor, const g EditorData *ed; EditorFlags flags = editor->flags; - if (EDITOR_ERRORS(flags)) return static_cast(EDITOR_ERRORS(flags)); + if (editor_errors(flags)) return static_cast(editor_errors(flags)); ed = g_new0(EditorData, 1); ed->list = filelist_copy(list); @@ -1327,7 +1327,7 @@ static EditorFlags editor_command_start(const EditorDescription *editor, const g editor_command_next_start(ed); /* errors from editor_command_next_start will be handled via callback */ - return static_cast(EDITOR_ERRORS(flags)); + return static_cast(editor_errors(flags)); } gboolean is_valid_editor_command(const gchar *key) @@ -1349,11 +1349,11 @@ EditorFlags start_editor_from_filelist_full(const gchar *key, GList *list, const error = editor_command_parse(editor, list, TRUE, nullptr); - if (EDITOR_ERRORS(error)) return error; + if (editor_errors(error)) return error; error = static_cast(error | editor_command_start(editor, editor->name, list, working_directory, cb, data)); - if (EDITOR_ERRORS(error)) + if (editor_errors(error)) { gchar *text = g_strdup_printf(_("%s\n\"%s\""), editor_get_error_str(error), editor->file); @@ -1361,7 +1361,7 @@ EditorFlags start_editor_from_filelist_full(const gchar *key, GList *list, const g_free(text); } - return static_cast(EDITOR_ERRORS(error)); + return static_cast(editor_errors(error)); } EditorFlags start_editor_from_filelist(const gchar *key, GList *list) diff --git a/src/editors.h b/src/editors.h index 2418de30..1b581d04 100644 --- a/src/editors.h +++ b/src/editors.h @@ -65,8 +65,8 @@ struct EditorDescription { gboolean disabled; /**< display disabled by user */ }; -#define EDITOR_ERRORS(flags) ((flags) & EDITOR_ERROR_MASK) -#define EDITOR_ERRORS_BUT_SKIPPED(flags) (!!(((flags) & EDITOR_ERROR_MASK) && !((flags) & EDITOR_ERROR_SKIPPED))) +inline gint editor_errors(EditorFlags flags) { return flags & EDITOR_ERROR_MASK; } +inline gboolean editor_errors_but_skipped(EditorFlags flags) { return !!(editor_errors(flags) && !(flags & EDITOR_ERROR_SKIPPED)); } /** diff --git a/src/history-list.cc b/src/history-list.cc index d65a70e6..abde2c49 100644 --- a/src/history-list.cc +++ b/src/history-list.cc @@ -465,7 +465,7 @@ void history_list_item_change(const gchar *key, const gchar *oldpath, const gcha { auto buf = static_cast(work->data); - if (!(g_str_has_prefix(buf, ".") && !newpath)) + if (!g_str_has_prefix(buf, ".") || newpath) { if (strcmp(buf, oldpath) == 0) { diff --git a/src/image.cc b/src/image.cc index 2e81edb5..c94b9ca5 100644 --- a/src/image.cc +++ b/src/image.cc @@ -1260,7 +1260,7 @@ void image_attach_window(ImageWindow *imd, GtkWidget *window, lw = layout_find_by_image(imd); - if (!(options->image.fit_window_to_image && lw && (lw->options.tools_float || lw->options.tools_hidden))) window = nullptr; + if (!options->image.fit_window_to_image || !lw || (!lw->options.tools_float && !lw->options.tools_hidden)) window = nullptr; pixbuf_renderer_set_parent(reinterpret_cast(imd->pr), reinterpret_cast(window)); diff --git a/src/layout-util.cc b/src/layout-util.cc index 6b42911d..2f743c68 100644 --- a/src/layout-util.cc +++ b/src/layout-util.cc @@ -3878,12 +3878,14 @@ static void layout_util_sync_views(LayoutWindow *lw) action = gtk_action_group_get_action(lw->action_group, "ConnectZoomMenu"); gtk_action_set_sensitive(action, lw->split_mode != SPLIT_NONE); + // @todo `which` is deprecated, use command -v + gboolean is_write_rotation = !runcmd("which exiftran >/dev/null 2>&1") + && !runcmd("which mogrify >/dev/null 2>&1") + && !options->metadata.write_orientation; action = gtk_action_group_get_action(lw->action_group, "WriteRotation"); - gtk_action_set_sensitive(action, !(runcmd("which exiftran >/dev/null 2>&1") || - runcmd("which mogrify >/dev/null 2>&1") || options->metadata.write_orientation)); + gtk_action_set_sensitive(action, is_write_rotation); action = gtk_action_group_get_action(lw->action_group, "WriteRotationKeepDate"); - gtk_action_set_sensitive(action, !(runcmd("which exiftran >/dev/null 2>&1") || - runcmd("which mogrify >/dev/null 2>&1") || options->metadata.write_orientation)); + gtk_action_set_sensitive(action, is_write_rotation); action = gtk_action_group_get_action(lw->action_group, "StereoAuto"); gtk_radio_action_set_current_value(GTK_RADIO_ACTION(action), layout_image_stereo_pixbuf_get(lw)); diff --git a/src/menu.cc b/src/menu.cc index 9ac99948..085e69de 100644 --- a/src/menu.cc +++ b/src/menu.cc @@ -83,7 +83,7 @@ static void add_edit_items(GtkWidget *menu, GCallback func, GList *fd_list) work = work->next; gboolean active = TRUE; - if (fd_list && EDITOR_ERRORS(editor_command_parse(editor, fd_list, FALSE, nullptr))) + if (fd_list && editor_errors(editor_command_parse(editor, fd_list, FALSE, nullptr))) active = FALSE; if (active) diff --git a/src/metadata.cc b/src/metadata.cc index 85b6995d..41d16802 100644 --- a/src/metadata.cc +++ b/src/metadata.cc @@ -48,6 +48,9 @@ struct ExifData; +namespace +{ + enum MetadataKey { MK_NONE, MK_KEYWORDS, @@ -58,7 +61,8 @@ enum MetadataKey { /** * @brief Tags that will be written to all files in a group - selected by: options->metadata.sync_grouped_files, Preferences/Metadata/Write The Same Description Tags To All Grouped Sidecars */ -static const gchar *group_keys[] = { +// @todo Use std::array +const gchar *group_keys[] = { "Xmp.dc.title", "Xmp.photoshop.Urgency", "Xmp.photoshop.Category", @@ -80,7 +84,19 @@ static const gchar *group_keys[] = { "Xmp.dc.rights", "Xmp.dc.description", "Xmp.photoshop.CaptionWriter", - nullptr}; + nullptr +}; + +inline gboolean is_keywords_separator(gchar c) +{ + return c == ',' + || c == ';' + || c == '\n' + || c == '\r' + || c == '\b'; +} + +} // namespace static gboolean metadata_write_queue_idle_cb(gpointer data); static gboolean metadata_legacy_write(FileData *fd); @@ -1014,8 +1030,6 @@ gchar *find_string_in_list(GList *list, const gchar *string) return find_string_in_list_utf8nocase(list, string); } -#define KEYWORDS_SEPARATOR(c) ((c) == ',' || (c) == ';' || (c) == '\n' || (c) == '\r' || (c) == '\b') - GList *string_to_keywords_list(const gchar *text) { GList *list = nullptr; @@ -1026,9 +1040,9 @@ GList *string_to_keywords_list(const gchar *text) const gchar *begin; gint l = 0; - while (KEYWORDS_SEPARATOR(*ptr)) ptr++; + while (is_keywords_separator(*ptr)) ptr++; begin = ptr; - while (*ptr != '\0' && !KEYWORDS_SEPARATOR(*ptr)) + while (*ptr != '\0' && !is_keywords_separator(*ptr)) { ptr++; l++; diff --git a/src/renderer-tiles.cc b/src/renderer-tiles.cc index 5e3068c5..377f142a 100644 --- a/src/renderer-tiles.cc +++ b/src/renderer-tiles.cc @@ -1559,7 +1559,7 @@ static void rt_tile_render(RendererTiles *rt, ImageTile *it, { cairo_t *cr; - if (pr->func_post_process && !(pr->post_process_slow && fast)) + if (pr->func_post_process && (!pr->post_process_slow || !fast)) pr->func_post_process(pr, &it->pixbuf, x, y, w, h, pr->post_process_user_data); cr = cairo_create(it->surface); diff --git a/src/utilops.cc b/src/utilops.cc index daf8b72c..f65dabaf 100644 --- a/src/utilops.cc +++ b/src/utilops.cc @@ -680,7 +680,7 @@ static gint file_util_perform_ci_cb(gpointer resume_data, EditorFlags flags, GLi ud->resume_data = resume_data; - if (EDITOR_ERRORS_BUT_SKIPPED(flags)) + if (editor_errors_but_skipped(flags)) { GString *msg = g_string_new(editor_get_error_str(flags)); GenericDialog *d; @@ -722,7 +722,7 @@ static gint file_util_perform_ci_cb(gpointer resume_data, EditorFlags flags, GLi auto fd = static_cast(list->data); list = list->next; - if (!EDITOR_ERRORS(flags)) /* files were successfully deleted, call the maint functions */ + if (!editor_errors(flags)) /* files were successfully deleted, call the maint functions */ { if (ud->with_sidecars) file_data_sc_apply_ci(fd); @@ -963,7 +963,7 @@ static void file_util_perform_ci_dir(UtilityData *ud, gboolean internal, gboolea static gint file_util_perform_ci_dir_cb(gpointer, EditorFlags flags, GList *, gpointer data) { auto ud = static_cast(data); - file_util_perform_ci_dir(ud, FALSE, !EDITOR_ERRORS_BUT_SKIPPED(flags)); + file_util_perform_ci_dir(ud, FALSE, !editor_errors_but_skipped(flags)); return EDITOR_CB_CONTINUE; /* does not matter, there was just single directory */ } @@ -1023,7 +1023,7 @@ void file_util_perform_ci(UtilityData *ud) } } - if (EDITOR_ERRORS(flags)) + if (editor_errors(flags)) { gchar *text = g_strdup_printf(_("%s\nUnable to start external command.\n"), editor_get_error_str(flags)); file_util_warning_dialog(ud->messages.fail, text, GQ_ICON_DIALOG_ERROR, nullptr); -- 2.20.1