From f8d981349e926bb24576315cb5669b381293916d Mon Sep 17 00:00:00 2001 From: Mike Gelfand Date: Mon, 14 Nov 2022 17:22:38 +0100 Subject: [PATCH] Fix issues reported by clang-tidy `misc` checks (GTK client) (#4167) * Fix `misc-const-correctness` clang-tidy issues * Fix `misc-no-recursion` clang-tidy issues * Extend clang-tidy configuration --- gtk/.clang-tidy | 2 ++ gtk/FileList.cc | 54 ++++++++++++++++++++++----------- gtk/Prefs.cc | 2 +- gtk/Session.cc | 6 ++-- gtk/Utils.cc | 80 +++++++++++++++++++++++++++---------------------- gtk/Utils.h | 2 +- 6 files changed, 88 insertions(+), 58 deletions(-) diff --git a/gtk/.clang-tidy b/gtk/.clang-tidy index 82148826f..f0358e522 100644 --- a/gtk/.clang-tidy +++ b/gtk/.clang-tidy @@ -5,6 +5,7 @@ Checks: > -cppcoreguidelines-avoid-magic-numbers, -cppcoreguidelines-avoid-non-const-global-variables, -cppcoreguidelines-narrowing-conversions, + misc-*, modernize-*, -modernize-use-trailing-return-type, performance-*, @@ -15,4 +16,5 @@ Checks: > -readability-redundant-access-specifiers CheckOptions: + misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic: true modernize-pass-by-value.ValuesOnly: true diff --git a/gtk/FileList.cc b/gtk/FileList.cc index 76c0306ce..cb3b04648 100644 --- a/gtk/FileList.cc +++ b/gtk/FileList.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -91,6 +92,7 @@ public: TR_DISABLE_COPY_MOVE(Impl) void set_torrent(tr_torrent_id_t torrent_id); + void reset_torrent(); private: void clearData(); @@ -269,26 +271,34 @@ bool refreshFilesForeach( return false; /* keep walking */ } -void gtr_tree_model_foreach_postorder_subtree( - Gtk::TreeModel::iterator const& parent, - Gtk::TreeModel::SlotForeachIter const& func) -{ - for (auto& child : parent->children()) - { - gtr_tree_model_foreach_postorder_subtree(TR_GTK_TREE_MODEL_CHILD_ITER(child), func); - } - - if (parent) - { - func(parent); - } -} - void gtr_tree_model_foreach_postorder(Glib::RefPtr const& model, Gtk::TreeModel::SlotForeachIter const& func) { - for (auto& iter : model->children()) + auto items = std::stack(); + if (auto const root_child_it = model->children().begin(); root_child_it) { - gtr_tree_model_foreach_postorder_subtree(TR_GTK_TREE_MODEL_CHILD_ITER(iter), func); + items.push(root_child_it); + } + + while (!items.empty()) + { + while (items.top()) + { + if (auto const child_it = items.top()->children().begin(); child_it) + { + items.push(child_it); + } + else + { + func(items.top()++); + } + } + + items.pop(); + + if (!items.empty()) + { + func(items.top()++); + } } } @@ -418,7 +428,7 @@ std::vector FileList::Impl::getActiveFilesForPath(Gtk::TreeMode void FileList::clear() { - impl_->set_torrent(-1); + impl_->reset_torrent(); } namespace @@ -566,6 +576,14 @@ void FileList::Impl::set_torrent(tr_torrent_id_t torrent_id) // view_->expand_all(); } +void FileList::Impl::reset_torrent() +{ + clearData(); + + store_ = Gtk::TreeStore::create(file_cols); + view_->set_model(store_); +} + /*** **** ***/ diff --git a/gtk/Prefs.cc b/gtk/Prefs.cc index 2ead02375..3e7fc428b 100644 --- a/gtk/Prefs.cc +++ b/gtk/Prefs.cc @@ -85,7 +85,7 @@ static void tr_prefs_init_defaults(tr_variant* d) static void ensure_sound_cmd_is_a_list(tr_variant* dict) { - tr_quark key = TR_KEY_torrent_complete_sound_command; + tr_quark const key = TR_KEY_torrent_complete_sound_command; tr_variant* list = nullptr; if (tr_variantDictFindList(dict, key, &list)) { diff --git a/gtk/Session.cc b/gtk/Session.cc index 3053cd76a..53f854133 100644 --- a/gtk/Session.cc +++ b/gtk/Session.cc @@ -942,7 +942,7 @@ bool is_torrent_active(tr_stat const* st) void Session::add_torrent(tr_torrent* tor, bool do_notify) { - ScopedModelSortBlocker disable_sort(*impl_->get_model().get()); + ScopedModelSortBlocker const disable_sort(*impl_->get_model().get()); impl_->add_torrent(tor, do_notify); } @@ -1032,7 +1032,7 @@ int Session::Impl::add_ctor(tr_ctor* ctor, bool do_prompt, bool do_notify) if (!do_prompt) { - ScopedModelSortBlocker disable_sort(*sorted_model_.get()); + ScopedModelSortBlocker const disable_sort(*sorted_model_.get()); add_torrent(create_new_torrent(ctor), do_notify); tr_ctorFree(ctor); return 0; @@ -1260,7 +1260,7 @@ void Session::load(bool force_paused) auto const n_torrents = tr_sessionLoadTorrents(session, ctor); tr_ctorFree(ctor); - ScopedModelSortBlocker disable_sort(*impl_->get_model().get()); + ScopedModelSortBlocker const disable_sort(*impl_->get_model().get()); auto torrents = std::vector{}; torrents.resize(n_torrents); diff --git a/gtk/Utils.cc b/gtk/Utils.cc index c2a2ca0c0..dbf83957e 100644 --- a/gtk/Utils.cc +++ b/gtk/Utils.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -585,45 +586,54 @@ auto const ChildHiddenKey = Glib::Quark("gtr-child-hidden"); } // namespace -void gtr_widget_set_visible(Gtk::Widget& w, bool b) +void gtr_widget_set_visible(Gtk::Widget& widget, bool is_visible) { - /* toggle the transient children, too */ - if (auto const* const window = dynamic_cast(&w); window != nullptr) + auto* const widget_as_window = dynamic_cast(&widget); + if (widget_as_window == nullptr) { - auto top_levels = Gtk::Window::list_toplevels(); - - for (auto top_level_it = top_levels.begin(); top_level_it != top_levels.end();) - { - auto* const l = *top_level_it++; - - if (l->get_transient_for() != window) - { - continue; - } - - if (l->get_visible() == b) - { - continue; - } - - if (b && l->get_data(ChildHiddenKey) != nullptr) - { - l->steal_data(ChildHiddenKey); - gtr_widget_set_visible(*l, true); - } - else if (!b) - { - l->set_data(ChildHiddenKey, GINT_TO_POINTER(1)); - gtr_widget_set_visible(*l, false); - - // Retrieve updated top-levels list in case hiding the window resulted in its destruction - top_levels = Gtk::Window::list_toplevels(); - top_level_it = top_levels.begin(); - } - } + widget.set_visible(is_visible); + return; } - w.set_visible(b); + /* toggle the transient children, too */ + auto windows = std::stack(); + windows.push(widget_as_window); + + while (!windows.empty()) + { + auto* const window = windows.top(); + bool transient_child_found = false; + + for (auto* const top_level_window : Gtk::Window::list_toplevels()) + { + if (top_level_window->get_transient_for() != window || top_level_window->get_visible() == is_visible) + { + continue; + } + + windows.push(top_level_window); + transient_child_found = true; + break; + } + + if (transient_child_found) + { + continue; + } + + if (is_visible && window->get_data(ChildHiddenKey) != nullptr) + { + window->steal_data(ChildHiddenKey); + window->set_visible(true); + } + else if (!is_visible) + { + window->set_data(ChildHiddenKey, GINT_TO_POINTER(1)); + window->set_visible(false); + } + + windows.pop(); + } } Gtk::Window& gtr_widget_get_window(Gtk::Widget& widget) diff --git a/gtk/Utils.h b/gtk/Utils.h index 72163b01d..a96cea762 100644 --- a/gtk/Utils.h +++ b/gtk/Utils.h @@ -177,7 +177,7 @@ Glib::ustring gtr_get_help_uri(); ***/ /* backwards-compatible wrapper around gtk_widget_set_visible() */ -void gtr_widget_set_visible(Gtk::Widget&, bool); +void gtr_widget_set_visible(Gtk::Widget& widget, bool is_visible); Gtk::Window& gtr_widget_get_window(Gtk::Widget& widget);