Enable and fix clang-tidy readability-simplify-boolean-expr

Replace some macros with inline functions.

diff --git a/.clang-tidy b/.clang-tidy
index c87a7f4..88cf994 100644 (file)
--- 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 9d39104..08cfdf6 100644 (file)
--- 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 9777e88..d290950 100644 (file)
--- 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 9daeaa3..43bba68 100644 (file)
--- 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<EditorFlags>(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<EditorFlags>(EDITOR_ERRORS(ed->flags));
+       return static_cast<EditorFlags>(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<EditorFlags>(EDITOR_ERRORS(ed->flags));
+               return static_cast<EditorFlags>(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<EditorFlags>(EDITOR_ERRORS(ed->flags));
+       flags = static_cast<EditorFlags>(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<EditorFlags>(EDITOR_ERRORS(flags));
+       if (editor_errors(flags)) return static_cast<EditorFlags>(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<EditorFlags>(EDITOR_ERRORS(flags));
+       return static_cast<EditorFlags>(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<EditorFlags>(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<EditorFlags>(EDITOR_ERRORS(error));
+       return static_cast<EditorFlags>(editor_errors(error));
 }
 
 EditorFlags start_editor_from_filelist(const gchar *key, GList *list)

diff --git a/src/editors.h b/src/editors.h
index 2418de3..1b581d0 100644 (file)
--- 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 d65a70e..abde2c4 100644 (file)
--- 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<gchar *>(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 2e81edb..c94b9ca 100644 (file)
--- 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<PixbufRenderer *>(imd->pr), reinterpret_cast<GtkWindow *>(window));
 

diff --git a/src/layout-util.cc b/src/layout-util.cc
index 6b42911..2f743c6 100644 (file)
--- 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 9ac9994..085e69d 100644 (file)
--- 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 85b6995..41d1680 100644 (file)
--- 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 5e3068c..377f142 100644 (file)
--- 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 daf8b72..f65daba 100644 (file)
--- 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<FileData *>(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<UtilityData *>(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);