From fa232ac1801e398bdd3d254e62c26a783de6a225 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 9 May 2019 04:56:52 +0200 Subject: [PATCH] LibGUI: Remove GModel activations to GAbstractView. Now you can hook activation via GAbstractView::on_activation. The design still isn't quite right, we should eventually move the selection away from the model somehow. --- Applications/FileManager/DirectoryView.cpp | 50 +++++++++++++++++++ Applications/FileManager/DirectoryView.h | 2 + Applications/IRCClient/IRCAppWindow.cpp | 4 +- .../IRCClient/IRCChannelMemberListModel.cpp | 7 --- .../IRCClient/IRCChannelMemberListModel.h | 3 -- Applications/IRCClient/IRCLogBufferModel.cpp | 4 -- Applications/IRCClient/IRCLogBufferModel.h | 1 - Applications/IRCClient/IRCWindow.cpp | 1 + Applications/IRCClient/IRCWindowListModel.cpp | 7 --- Applications/IRCClient/IRCWindowListModel.h | 1 - LibGUI/GAbstractView.cpp | 6 +++ LibGUI/GAbstractView.h | 6 +++ LibGUI/GDirectoryModel.cpp | 39 --------------- LibGUI/GDirectoryModel.h | 14 +++--- LibGUI/GFilePicker.cpp | 18 ++++++- LibGUI/GFilePicker.h | 5 +- LibGUI/GFileSystemModel.cpp | 4 -- LibGUI/GFileSystemModel.h | 1 - LibGUI/GItemView.cpp | 4 +- LibGUI/GModel.cpp | 2 - LibGUI/GModel.h | 5 -- LibGUI/GSortingProxyModel.cpp | 5 -- LibGUI/GSortingProxyModel.h | 1 - LibGUI/GTableView.cpp | 19 ++++--- LibGUI/GTreeView.cpp | 2 + 25 files changed, 107 insertions(+), 104 deletions(-) diff --git a/Applications/FileManager/DirectoryView.cpp b/Applications/FileManager/DirectoryView.cpp index a6177b1462b..52659431291 100644 --- a/Applications/FileManager/DirectoryView.cpp +++ b/Applications/FileManager/DirectoryView.cpp @@ -1,5 +1,47 @@ #include "DirectoryView.h" #include +#include +#include +#include + +void DirectoryView::handle_activation(const GModelIndex& index) +{ + if (!index.is_valid()) + return; + dbgprintf("on activation: %d,%d, this=%p, m_model=%p\n", index.row(), index.column(), this, m_model.ptr()); + auto& entry = model().entry(index.row()); + FileSystemPath path(String::format("%s/%s", model().path().characters(), entry.name.characters())); + if (entry.is_directory()) { + open(path.string()); + return; + } + if (entry.is_executable()) { + if (fork() == 0) { + int rc = execl(path.string().characters(), path.string().characters(), nullptr); + if (rc < 0) + perror("exec"); + ASSERT_NOT_REACHED(); + } + return; + } + + if (path.string().to_lowercase().ends_with(".png")) { + if (fork() == 0) { + int rc = execl("/bin/qs", "/bin/qs", path.string().characters(), nullptr); + if (rc < 0) + perror("exec"); + ASSERT_NOT_REACHED(); + } + return; + } + + if (fork() == 0) { + int rc = execl("/bin/TextEditor", "/bin/TextEditor", path.string().characters(), nullptr); + if (rc < 0) + perror("exec"); + ASSERT_NOT_REACHED(); + } +}; DirectoryView::DirectoryView(GWidget* parent) : GStackWidget(parent) @@ -34,6 +76,14 @@ DirectoryView::DirectoryView(GWidget* parent) on_thumbnail_progress(done, total); }; + m_item_view->on_activation = [&] (const GModelIndex& index) { + handle_activation(index); + }; + m_table_view->on_activation = [&] (auto& index) { + auto& filter_model = (GSortingProxyModel&)*m_table_view->model(); + handle_activation(filter_model.map_to_target(index)); + }; + set_view_mode(ViewMode::Icon); } diff --git a/Applications/FileManager/DirectoryView.h b/Applications/FileManager/DirectoryView.h index f657999a7b8..dec41cf1d14 100644 --- a/Applications/FileManager/DirectoryView.h +++ b/Applications/FileManager/DirectoryView.h @@ -29,6 +29,8 @@ private: GDirectoryModel& model() { return *m_model; } const GDirectoryModel& model() const { return *m_model; } + void handle_activation(const GModelIndex&); + void set_status_message(const String&); ViewMode m_view_mode { Invalid }; diff --git a/Applications/IRCClient/IRCAppWindow.cpp b/Applications/IRCClient/IRCAppWindow.cpp index c37a5527c54..20ee953f5a6 100644 --- a/Applications/IRCClient/IRCAppWindow.cpp +++ b/Applications/IRCClient/IRCAppWindow.cpp @@ -151,9 +151,11 @@ void IRCAppWindow::setup_widgets() m_window_list->set_headers_visible(false); m_window_list->set_alternating_row_colors(false); m_window_list->set_model(m_client.client_window_list_model()); + m_window_list->set_activates_on_selection(true); m_window_list->set_size_policy(SizePolicy::Fixed, SizePolicy::Fill); m_window_list->set_preferred_size({ 100, 0 }); - m_client.client_window_list_model()->on_activation = [this] (IRCWindow& window) { + m_window_list->on_activation = [this] (auto& index) { + auto& window = m_client.window_at(index.row()); m_container->set_active_widget(&window); window.clear_unread_count(); }; diff --git a/Applications/IRCClient/IRCChannelMemberListModel.cpp b/Applications/IRCClient/IRCChannelMemberListModel.cpp index 057838a0465..25fed4cbf3c 100644 --- a/Applications/IRCClient/IRCChannelMemberListModel.cpp +++ b/Applications/IRCClient/IRCChannelMemberListModel.cpp @@ -6,7 +6,6 @@ IRCChannelMemberListModel::IRCChannelMemberListModel(IRCChannel& channel) : m_channel(channel) { - set_activates_on_selection(true); } IRCChannelMemberListModel::~IRCChannelMemberListModel() @@ -53,9 +52,3 @@ void IRCChannelMemberListModel::update() { did_update(); } - -void IRCChannelMemberListModel::activate(const GModelIndex& index) -{ - if (on_activation) - on_activation(m_channel.member_at(index.row())); -} diff --git a/Applications/IRCClient/IRCChannelMemberListModel.h b/Applications/IRCClient/IRCChannelMemberListModel.h index 8b6c4797630..2f67cf15caf 100644 --- a/Applications/IRCClient/IRCChannelMemberListModel.h +++ b/Applications/IRCClient/IRCChannelMemberListModel.h @@ -17,9 +17,6 @@ public: virtual ColumnMetadata column_metadata(int column) const override; virtual GVariant data(const GModelIndex&, Role = Role::Display) const override; virtual void update() override; - virtual void activate(const GModelIndex&) override; - - Function on_activation; private: explicit IRCChannelMemberListModel(IRCChannel&); diff --git a/Applications/IRCClient/IRCLogBufferModel.cpp b/Applications/IRCClient/IRCLogBufferModel.cpp index 1e451770f21..518ad63f3c9 100644 --- a/Applications/IRCClient/IRCLogBufferModel.cpp +++ b/Applications/IRCClient/IRCLogBufferModel.cpp @@ -72,7 +72,3 @@ void IRCLogBufferModel::update() { did_update(); } - -void IRCLogBufferModel::activate(const GModelIndex&) -{ -} diff --git a/Applications/IRCClient/IRCLogBufferModel.h b/Applications/IRCClient/IRCLogBufferModel.h index d4f29740b14..5be8237d075 100644 --- a/Applications/IRCClient/IRCLogBufferModel.h +++ b/Applications/IRCClient/IRCLogBufferModel.h @@ -22,7 +22,6 @@ public: virtual ColumnMetadata column_metadata(int column) const override; virtual GVariant data(const GModelIndex&, Role = Role::Display) const override; virtual void update() override; - virtual void activate(const GModelIndex&) override; private: explicit IRCLogBufferModel(Retained&&); diff --git a/Applications/IRCClient/IRCWindow.cpp b/Applications/IRCClient/IRCWindow.cpp index ea00c7801d1..7364707e823 100644 --- a/Applications/IRCClient/IRCWindow.cpp +++ b/Applications/IRCClient/IRCWindow.cpp @@ -37,6 +37,7 @@ IRCWindow::IRCWindow(IRCClient& client, void* owner, Type type, const String& na member_view->set_preferred_size({ 100, 0 }); member_view->set_alternating_row_colors(false); member_view->set_model(channel().member_model()); + member_view->set_activates_on_selection(true); } m_text_editor = new GTextEditor(GTextEditor::SingleLine, this); diff --git a/Applications/IRCClient/IRCWindowListModel.cpp b/Applications/IRCClient/IRCWindowListModel.cpp index 2adb2ef360e..bc089c2e0cd 100644 --- a/Applications/IRCClient/IRCWindowListModel.cpp +++ b/Applications/IRCClient/IRCWindowListModel.cpp @@ -8,7 +8,6 @@ IRCWindowListModel::IRCWindowListModel(IRCClient& client) : m_client(client) { - set_activates_on_selection(true); } IRCWindowListModel::~IRCWindowListModel() @@ -72,9 +71,3 @@ void IRCWindowListModel::update() { did_update(); } - -void IRCWindowListModel::activate(const GModelIndex& index) -{ - if (on_activation) - on_activation(m_client.window_at(index.row())); -} diff --git a/Applications/IRCClient/IRCWindowListModel.h b/Applications/IRCClient/IRCWindowListModel.h index 607658cd663..d565eb78e30 100644 --- a/Applications/IRCClient/IRCWindowListModel.h +++ b/Applications/IRCClient/IRCWindowListModel.h @@ -21,7 +21,6 @@ public: virtual ColumnMetadata column_metadata(int column) const override; virtual GVariant data(const GModelIndex&, Role = Role::Display) const override; virtual void update() override; - virtual void activate(const GModelIndex&) override; Function on_activation; diff --git a/LibGUI/GAbstractView.cpp b/LibGUI/GAbstractView.cpp index 924cc0449f3..d9088f4afac 100644 --- a/LibGUI/GAbstractView.cpp +++ b/LibGUI/GAbstractView.cpp @@ -88,3 +88,9 @@ void GAbstractView::stop_editing() delete m_edit_widget; m_edit_widget = nullptr; } + +void GAbstractView::activate(const GModelIndex& index) +{ + if (on_activation) + on_activation(index); +} diff --git a/LibGUI/GAbstractView.h b/LibGUI/GAbstractView.h index f660e141cef..1660e40dffd 100644 --- a/LibGUI/GAbstractView.h +++ b/LibGUI/GAbstractView.h @@ -27,6 +27,10 @@ public: void begin_editing(const GModelIndex&); void stop_editing(); + void set_activates_on_selection(bool b) { m_activates_on_selection = b; } + bool activates_on_selection() const { return m_activates_on_selection; } + + Function on_activation; Function on_model_notification; virtual const char* class_name() const override { return "GAbstractView"; } @@ -34,6 +38,7 @@ public: protected: virtual void model_notification(const GModelNotification&); virtual void did_scroll() override; + void activate(const GModelIndex&); void update_edit_widget_position(); bool m_editable { false }; @@ -43,4 +48,5 @@ protected: private: RetainPtr m_model; + bool m_activates_on_selection { false }; }; diff --git a/LibGUI/GDirectoryModel.cpp b/LibGUI/GDirectoryModel.cpp index 9c0c32570f9..18f539a56e4 100644 --- a/LibGUI/GDirectoryModel.cpp +++ b/LibGUI/GDirectoryModel.cpp @@ -285,42 +285,3 @@ void GDirectoryModel::open(const String& a_path) update(); set_selected_index(index(0, 0)); } - -void GDirectoryModel::activate(const GModelIndex& index) -{ - if (!index.is_valid()) - return; - auto& entry = this->entry(index.row()); - FileSystemPath path(String::format("%s/%s", m_path.characters(), entry.name.characters())); - if (entry.is_directory()) { - open(path.string()); - return; - } - if (entry.is_executable()) { - if (fork() == 0) { - int rc = execl(path.string().characters(), path.string().characters(), nullptr); - if (rc < 0) - perror("exec"); - ASSERT_NOT_REACHED(); - } - return; - } - - if (path.string().to_lowercase().ends_with(".png")) { - if (fork() == 0) { - int rc = execl("/bin/qs", "/bin/qs", path.string().characters(), nullptr); - if (rc < 0) - perror("exec"); - ASSERT_NOT_REACHED(); - } - return; - } - - if (fork() == 0) { - int rc = execl("/bin/TextEditor", "/bin/TextEditor", path.string().characters(), nullptr); - if (rc < 0) - perror("exec"); - ASSERT_NOT_REACHED(); - } - return; -} diff --git a/LibGUI/GDirectoryModel.h b/LibGUI/GDirectoryModel.h index 7bc5d38aaaa..e33693ed455 100644 --- a/LibGUI/GDirectoryModel.h +++ b/LibGUI/GDirectoryModel.h @@ -27,7 +27,6 @@ public: virtual ColumnMetadata column_metadata(int column) const override; virtual GVariant data(const GModelIndex&, Role = Role::Display) const override; virtual void update() override; - virtual void activate(const GModelIndex&) override; String path() const { return m_path; } void open(const String& path); @@ -35,12 +34,6 @@ public: Function on_thumbnail_progress; -private: - GDirectoryModel(); - - String name_for_uid(uid_t) const; - String name_for_gid(gid_t) const; - struct Entry { String name; size_t size { 0 }; @@ -60,6 +53,13 @@ private: return m_directories[index]; return m_files[index - m_directories.size()]; } + +private: + GDirectoryModel(); + + String name_for_uid(uid_t) const; + String name_for_gid(gid_t) const; + GIcon icon_for(const Entry& entry) const; String m_path; diff --git a/LibGUI/GFilePicker.cpp b/LibGUI/GFilePicker.cpp index 8686997bf02..bc54d99601e 100644 --- a/LibGUI/GFilePicker.cpp +++ b/LibGUI/GFilePicker.cpp @@ -4,6 +4,8 @@ #include #include #include +#include +#include GFilePicker::GFilePicker(const String& path, CObject* parent) : GDialog(parent) @@ -18,8 +20,8 @@ GFilePicker::GFilePicker(const String& path, CObject* parent) main_widget()->set_fill_with_background_color(true); main_widget()->set_background_color(Color::LightGray); m_view = new GTableView(main_widget()); - m_view->set_model(*m_model); - model().open("/"); + m_view->set_model(GSortingProxyModel::create(*m_model)); + m_model->open(path); auto* lower_container = new GWidget(main_widget()); lower_container->set_layout(make(Orientation::Vertical)); @@ -37,6 +39,18 @@ GFilePicker::GFilePicker(const String& path, CObject* parent) filename_label->set_preferred_size({ 60, 0 }); auto* filename_textbox = new GTextBox(filename_container); + m_view->on_activation = [&] (auto& index) { + auto& filter_model = (GSortingProxyModel&)*m_view->model(); + auto local_index = filter_model.map_to_target(index); + const GDirectoryModel::Entry& entry = m_model->entry(local_index.row()); + + FileSystemPath path(String::format("%s/%s", m_model->path().characters(), entry.name.characters())); + + if (entry.is_directory()) + m_model->open(path.string()); + filename_textbox->set_text(entry.name); + }; + auto* button_container = new GWidget(lower_container); button_container->set_size_policy(SizePolicy::Fill, SizePolicy::Fixed); button_container->set_preferred_size({ 0, 20 }); diff --git a/LibGUI/GFilePicker.h b/LibGUI/GFilePicker.h index e470d712278..e61bae3dd27 100644 --- a/LibGUI/GFilePicker.h +++ b/LibGUI/GFilePicker.h @@ -8,11 +8,12 @@ public: GFilePicker(const String& path = "/", CObject* parent = nullptr); virtual ~GFilePicker() override; + String selected_file() const; + virtual const char* class_name() const override { return "GFilePicker"; } private: - GDirectoryModel& model() { return *m_model; } - GTableView* m_view { nullptr }; Retained m_model; + String m_selected_file; }; diff --git a/LibGUI/GFileSystemModel.cpp b/LibGUI/GFileSystemModel.cpp index 71d0dddc9ba..fbef0418bca 100644 --- a/LibGUI/GFileSystemModel.cpp +++ b/LibGUI/GFileSystemModel.cpp @@ -199,10 +199,6 @@ GVariant GFileSystemModel::data(const GModelIndex& index, Role role) const return { }; } -void GFileSystemModel::activate(const GModelIndex&) -{ -} - int GFileSystemModel::column_count(const GModelIndex&) const { return 1; diff --git a/LibGUI/GFileSystemModel.h b/LibGUI/GFileSystemModel.h index f553bfbcad3..7eb7f5ef0c3 100644 --- a/LibGUI/GFileSystemModel.h +++ b/LibGUI/GFileSystemModel.h @@ -23,7 +23,6 @@ public: virtual void update() override; virtual GModelIndex parent_index(const GModelIndex&) const override; virtual GModelIndex index(int row, int column = 0, const GModelIndex& parent = GModelIndex()) const override; - virtual void activate(const GModelIndex&) override; private: GFileSystemModel(const String& root_path, Mode); diff --git a/LibGUI/GItemView.cpp b/LibGUI/GItemView.cpp index f73d14555ba..71b0791f911 100644 --- a/LibGUI/GItemView.cpp +++ b/LibGUI/GItemView.cpp @@ -90,7 +90,7 @@ void GItemView::doubleclick_event(GMouseEvent& event) return; if (event.button() == GMouseButton::Left) { mousedown_event(event); - model()->activate(model()->selected_index()); + activate(model()->selected_index()); } } @@ -161,7 +161,7 @@ void GItemView::keydown_event(GKeyEvent& event) auto& model = *this->model(); if (event.key() == KeyCode::Key_Return) { - model.activate(model.selected_index()); + activate(model.selected_index()); return; } if (event.key() == KeyCode::Key_Home) { diff --git a/LibGUI/GModel.cpp b/LibGUI/GModel.cpp index 0447b27d02b..454d9996d5b 100644 --- a/LibGUI/GModel.cpp +++ b/LibGUI/GModel.cpp @@ -44,8 +44,6 @@ void GModel::set_selected_index(const GModelIndex& index) for_each_view([] (auto& view) { view.did_update_selection(); }); - if (m_activates_on_selection && is_valid(index)) - activate(index); } GModelIndex GModel::create_index(int row, int column, void* data) const diff --git a/LibGUI/GModel.h b/LibGUI/GModel.h index 3cc91f491be..9f3e087c3cc 100644 --- a/LibGUI/GModel.h +++ b/LibGUI/GModel.h @@ -56,7 +56,6 @@ public: virtual void update() = 0; virtual GModelIndex parent_index(const GModelIndex&) const { return { }; } virtual GModelIndex index(int row, int column = 0, const GModelIndex& = GModelIndex()) const { return create_index(row, column); } - virtual void activate(const GModelIndex&) { } virtual GModelIndex sibling(int row, int column, const GModelIndex& parent) const; virtual bool is_editable(const GModelIndex&) const { return false; } virtual void set_data(const GModelIndex&, const GVariant&) { } @@ -69,9 +68,6 @@ public: void set_selected_index(const GModelIndex&); GModelIndex selected_index() const { return m_selected_index; } - bool activates_on_selection() const { return m_activates_on_selection; } - void set_activates_on_selection(bool b) { m_activates_on_selection = b; } - virtual int key_column() const { return -1; } virtual GSortOrder sort_order() const { return GSortOrder::None; } virtual void set_key_column_and_sort_order(int, GSortOrder) { } @@ -93,7 +89,6 @@ protected: private: HashTable m_views; GModelIndex m_selected_index; - bool m_activates_on_selection { false }; }; inline GModelIndex GModelIndex::parent() const diff --git a/LibGUI/GSortingProxyModel.cpp b/LibGUI/GSortingProxyModel.cpp index a2fbe054c6b..380748fc49a 100644 --- a/LibGUI/GSortingProxyModel.cpp +++ b/LibGUI/GSortingProxyModel.cpp @@ -55,11 +55,6 @@ GVariant GSortingProxyModel::data(const GModelIndex& index, Role role) const return target().data(map_to_target(index), role); } -void GSortingProxyModel::activate(const GModelIndex& index) -{ - target().activate(map_to_target(index)); -} - void GSortingProxyModel::update() { target().update(); diff --git a/LibGUI/GSortingProxyModel.h b/LibGUI/GSortingProxyModel.h index 0288f3a9053..334878bcc5f 100644 --- a/LibGUI/GSortingProxyModel.h +++ b/LibGUI/GSortingProxyModel.h @@ -14,7 +14,6 @@ public: virtual ColumnMetadata column_metadata(int) const override; virtual GVariant data(const GModelIndex&, Role = Role::Display) const override; virtual void update() override; - virtual void activate(const GModelIndex&) override; virtual int key_column() const override { return m_key_column; } virtual GSortOrder sort_order() const override { return m_sort_order; } diff --git a/LibGUI/GTableView.cpp b/LibGUI/GTableView.cpp index 2a47d6820c0..b328d8329b3 100644 --- a/LibGUI/GTableView.cpp +++ b/LibGUI/GTableView.cpp @@ -63,7 +63,8 @@ Rect GTableView::row_rect(int item_index) const int GTableView::column_width(int column_index) const { - ASSERT(column_index >= 0 && column_index < model()->column_count()); + if (!model()) + return 0; auto& column_data = this->column_data(column_index); if (!column_data.has_initialized_width) { column_data.has_initialized_width = true; @@ -74,7 +75,8 @@ int GTableView::column_width(int column_index) const Rect GTableView::header_rect(int column_index) const { - ASSERT(column_index >= 0 && column_index < model()->column_count()); + if (!model()) + return { }; if (is_column_hidden(column_index)) return { }; int x_offset = 0; @@ -93,7 +95,8 @@ Point GTableView::adjusted_position(const Point& position) Rect GTableView::column_resize_grabbable_rect(int column) const { - ASSERT(column >= 0 && column < model()->column_count()); + if (!model()) + return { }; auto header_rect = this->header_rect(column); return { header_rect.right() - 1, header_rect.top(), 4, header_rect.height() }; } @@ -314,7 +317,7 @@ void GTableView::keydown_event(GKeyEvent& event) return; auto& model = *this->model(); if (event.key() == KeyCode::Key_Return) { - model.activate(model.selected_index()); + activate(model.selected_index()); return; } if (event.key() == KeyCode::Key_Up) { @@ -374,22 +377,18 @@ void GTableView::scroll_into_view(const GModelIndex& index, Orientation orientat GTableView::ColumnData& GTableView::column_data(int column) const { - ASSERT(model()); - ASSERT(column >= 0 && column < model()->column_count()); if (column >= m_column_data.size()) - m_column_data.resize(model()->column_count()); + m_column_data.resize(column + 1); return m_column_data.at(column); } bool GTableView::is_column_hidden(int column) const { - ASSERT(column >= 0 && column < model()->column_count()); return !column_data(column).visibility; } void GTableView::set_column_hidden(int column, bool hidden) { - ASSERT(column >= 0 && column < model()->column_count()); auto& column_data = this->column_data(column); if (column_data.visibility == !hidden) return; @@ -409,7 +408,7 @@ void GTableView::doubleclick_event(GMouseEvent& event) if (is_editable()) begin_editing(model.selected_index()); else - model.activate(model.selected_index()); + activate(model.selected_index()); } } } diff --git a/LibGUI/GTreeView.cpp b/LibGUI/GTreeView.cpp index c857c92e91d..0b7203deb54 100644 --- a/LibGUI/GTreeView.cpp +++ b/LibGUI/GTreeView.cpp @@ -243,6 +243,8 @@ void GTreeView::did_update_selection() if (opened_any) update_content_size(); update(); + if (activates_on_selection()) + activate(index); } void GTreeView::update_content_size()