Skip to content

Commit

Permalink
LibGUI: Don't require passing model to FileSystemModel::Node APIs
Browse files Browse the repository at this point in the history 
The Node API was obnoxiously requiring you to pass the model into it
all the time, simply because nodes could not find their way back to
the containing model. This patch adds a back-reference to the model
and simplifies the API.
  • Loading branch information
@awesomekling
awesomekling committed Aug 17, 2020
1 parent 38d8426 commit f034932
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 40 deletions.
2 changes: 1 addition & 1 deletion Applications/FileManager/DirectoryView.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ void DirectoryView::handle_activation(const GUI::ModelIndex& index)
return;
dbgprintf("on activation: %d,%d, this=%p, m_model=%p\n", index.row(), index.column(), this, m_model.ptr());
auto& node = model().node(index);
auto path = node.full_path(model());
auto path = node.full_path();

struct stat st;
if (stat(path.characters(), &st) < 0) {
Expand Down
8 changes: 4 additions & 4 deletions Applications/FileManager/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,11 +705,11 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
auto& node = directory_view.model().node(index);

if (node.is_directory()) {
auto should_get_enabled = access(node.full_path(directory_view.model()).characters(), W_OK) == 0 && GUI::Clipboard::the().type() == "text/uri-list";
auto should_get_enabled = access(node.full_path().characters(), W_OK) == 0 && GUI::Clipboard::the().type() == "text/uri-list";
folder_specific_paste_action->set_enabled(should_get_enabled);
directory_context_menu->popup(event.screen_position());
} else {
auto full_path = node.full_path(directory_view.model());
auto full_path = node.full_path();
current_file_handlers = directory_view.get_launch_handlers(full_path);

file_context_menu = GUI::Menu::construct("Directory View File");
Expand Down Expand Up @@ -774,10 +774,10 @@ int run_in_windowed_mode(RefPtr<Core::ConfigFile> config, String initial_locatio
return;

for (auto& url_to_copy : urls) {
if (!url_to_copy.is_valid() || url_to_copy.path() == target_node.full_path(directory_view.model()))
if (!url_to_copy.is_valid() || url_to_copy.path() == target_node.full_path())
continue;
auto new_path = String::format("%s/%s",
target_node.full_path(directory_view.model()).characters(),
target_node.full_path().characters(),
LexicalPath(url_to_copy.path()).basename().characters());

if (url_to_copy.path() == new_path)
Expand Down
4 changes: 2 additions & 2 deletions Libraries/LibGUI/FilePicker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ FilePicker::FilePicker(Window* parent_window, Mode mode, Options options, const
auto& filter_model = (SortingProxyModel&)*m_view->model();
auto local_index = filter_model.map_to_source(index);
const FileSystemModel::Node& node = m_model->node(local_index);
LexicalPath path { node.full_path(m_model) };
LexicalPath path { node.full_path() };

if (have_preview())
clear_preview();
Expand Down Expand Up @@ -240,7 +240,7 @@ FilePicker::FilePicker(Window* parent_window, Mode mode, Options options, const
auto& filter_model = (SortingProxyModel&)*m_view->model();
auto local_index = filter_model.map_to_source(index);
const FileSystemModel::Node& node = m_model->node(local_index);
auto path = node.full_path(m_model);
auto path = node.full_path();

if (node.is_directory()) {
set_path(path);
Expand Down
58 changes: 29 additions & 29 deletions Libraries/LibGUI/FileSystemModel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@

namespace GUI {

ModelIndex FileSystemModel::Node::index(const FileSystemModel& model, int column) const
ModelIndex FileSystemModel::Node::index(int column) const
{
if (!parent)
return {};
for (size_t row = 0; row < parent->children.size(); ++row) {
if (&parent->children[row] == this)
return model.create_index(row, column, const_cast<Node*>(this));
return m_model.create_index(row, column, const_cast<Node*>(this));
}
ASSERT_NOT_REACHED();
}
Expand Down Expand Up @@ -85,15 +85,15 @@ bool FileSystemModel::Node::fetch_data(const String& full_path, bool is_root)
return true;
}

void FileSystemModel::Node::traverse_if_needed(const FileSystemModel& model)
void FileSystemModel::Node::traverse_if_needed()
{
if (!is_directory() || has_traversed)
return;
has_traversed = true;
total_size = 0;

auto full_path = this->full_path(model);
Core::DirIterator di(full_path, model.should_show_dotfiles() ? Core::DirIterator::SkipParentAndBaseDir : Core::DirIterator::SkipDots);
auto full_path = this->full_path();
Core::DirIterator di(full_path, m_model.should_show_dotfiles() ? Core::DirIterator::SkipParentAndBaseDir : Core::DirIterator::SkipDots);
if (di.has_error()) {
m_error = di.error();
fprintf(stderr, "DirIterator: %s\n", di.error_string());
Expand All @@ -108,11 +108,11 @@ void FileSystemModel::Node::traverse_if_needed(const FileSystemModel& model)

for (auto& name : child_names) {
String child_path = String::format("%s/%s", full_path.characters(), name.characters());
NonnullOwnPtr<Node> child = make<Node>();
auto child = adopt_own(*new Node(m_model));
bool ok = child->fetch_data(child_path, false);
if (!ok)
continue;
if (model.m_mode == DirectoriesOnly && !S_ISDIR(child->mode))
if (m_model.m_mode == DirectoriesOnly && !S_ISDIR(child->mode))
continue;
child->name = name;
child->parent = this;
Expand All @@ -131,35 +131,35 @@ void FileSystemModel::Node::traverse_if_needed(const FileSystemModel& model)
fcntl(m_watch_fd, F_SETFD, FD_CLOEXEC);
dbg() << "Watching " << full_path << " for changes, m_watch_fd = " << m_watch_fd;
m_notifier = Core::Notifier::construct(m_watch_fd, Core::Notifier::Event::Read);
m_notifier->on_ready_to_read = [this, &model] {
m_notifier->on_ready_to_read = [this] {
char buffer[32];
int rc = read(m_notifier->fd(), buffer, sizeof(buffer));
ASSERT(rc >= 0);

has_traversed = false;
mode = 0;
children.clear();
reify_if_needed(model);
const_cast<FileSystemModel&>(model).did_update();
reify_if_needed();
m_model.did_update();
};
}

void FileSystemModel::Node::reify_if_needed(const FileSystemModel& model)
void FileSystemModel::Node::reify_if_needed()
{
traverse_if_needed(model);
traverse_if_needed();
if (mode != 0)
return;
fetch_data(full_path(model), parent == nullptr);
fetch_data(full_path(), parent == nullptr);
}

String FileSystemModel::Node::full_path(const FileSystemModel& model) const
String FileSystemModel::Node::full_path() const
{
Vector<String, 32> lineage;
for (auto* ancestor = parent; ancestor; ancestor = ancestor->parent) {
lineage.append(ancestor->name);
}
StringBuilder builder;
builder.append(model.root_path());
builder.append(m_model.root_path());
for (int i = lineage.size() - 1; i >= 0; --i) {
builder.append('/');
builder.append(lineage[i]);
Expand All @@ -174,17 +174,17 @@ ModelIndex FileSystemModel::index(const StringView& path, int column) const
LexicalPath lexical_path(path);
const Node* node = m_root;
if (lexical_path.string() == "/")
return m_root->index(*this, column);
return m_root->index(column);
for (size_t i = 0; i < lexical_path.parts().size(); ++i) {
auto& part = lexical_path.parts()[i];
bool found = false;
for (auto& child : node->children) {
if (child.name == part) {
const_cast<Node&>(child).reify_if_needed(*this);
const_cast<Node&>(child).reify_if_needed();
node = &child;
found = true;
if (i == lexical_path.parts().size() - 1)
return child.index(*this, column);
return child.index(column);
break;
}
}
Expand All @@ -197,8 +197,8 @@ ModelIndex FileSystemModel::index(const StringView& path, int column) const
String FileSystemModel::full_path(const ModelIndex& index) const
{
auto& node = this->node(index);
const_cast<Node&>(node).reify_if_needed(*this);
return node.full_path(*this);
const_cast<Node&>(node).reify_if_needed();
return node.full_path();
}

FileSystemModel::FileSystemModel(const StringView& root_path, Mode mode)
Expand Down Expand Up @@ -303,16 +303,16 @@ void FileSystemModel::set_root_path(const StringView& root_path)

void FileSystemModel::update()
{
m_root = make<Node>();
m_root->reify_if_needed(*this);
m_root = adopt_own(*new Node(*this));
m_root->reify_if_needed();

did_update();
}

int FileSystemModel::row_count(const ModelIndex& index) const
{
Node& node = const_cast<Node&>(this->node(index));
node.reify_if_needed(*this);
node.reify_if_needed();
if (node.is_directory())
return node.children.size();
return 0;
Expand All @@ -331,7 +331,7 @@ ModelIndex FileSystemModel::index(int row, int column, const ModelIndex& parent)
if (row < 0 || column < 0)
return {};
auto& node = this->node(parent);
const_cast<Node&>(node).reify_if_needed(*this);
const_cast<Node&>(node).reify_if_needed();
if (static_cast<size_t>(row) >= node.children.size())
return {};
return create_index(row, column, &node.children[row]);
Expand All @@ -346,7 +346,7 @@ ModelIndex FileSystemModel::parent_index(const ModelIndex& index) const
ASSERT(&node == m_root);
return {};
}
return node.parent->index(*this, index.column());
return node.parent->index(index.column());
}

Variant FileSystemModel::data(const ModelIndex& index, ModelRole role) const
Expand Down Expand Up @@ -377,14 +377,14 @@ Variant FileSystemModel::data(const ModelIndex& index, ModelRole role) const
if (role == ModelRole::Custom) {
// For GUI::FileSystemModel, custom role means the full path.
ASSERT(index.column() == Column::Name);
return node.full_path(*this);
return node.full_path();
}

if (role == ModelRole::DragData) {
if (index.column() == Column::Name) {
StringBuilder builder;
builder.append("file:https://");
builder.append(node.full_path(*this));
builder.append(node.full_path());
return builder.to_string();
}
return {};
Expand Down Expand Up @@ -454,7 +454,7 @@ Icon FileSystemModel::icon_for(const Node& node) const
}

if (node.is_directory()) {
if (node.full_path(*this) == Core::StandardPaths::home_directory()) {
if (node.full_path() == Core::StandardPaths::home_directory()) {
if (node.is_selected())
return FileIconProvider::home_directory_open_icon();
return FileIconProvider::home_directory_icon();
Expand Down Expand Up @@ -489,7 +489,7 @@ bool FileSystemModel::fetch_thumbnail_for(const Node& node)
{
// See if we already have the thumbnail
// we're looking for in the cache.
auto path = node.full_path(*this);
auto path = node.full_path();
auto it = s_thumbnail_cache.find(path);
if (it != s_thumbnail_cache.end()) {
if (!(*it).value)
Expand Down
15 changes: 11 additions & 4 deletions Libraries/LibGUI/FileSystemModel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,18 @@ class FileSystemModel
int error() const { return m_error; }
const char* error_string() const { return strerror(m_error); }

String full_path(const FileSystemModel&) const;
String full_path() const;

private:
friend class FileSystemModel;

explicit Node(FileSystemModel& model)
: m_model(model)
{
}

FileSystemModel& m_model;

Node* parent { nullptr };
NonnullOwnPtrVector<Node> children;
bool has_traversed { false };
Expand All @@ -103,9 +110,9 @@ class FileSystemModel

int m_error { 0 };

ModelIndex index(const FileSystemModel&, int column) const;
void traverse_if_needed(const FileSystemModel&);
void reify_if_needed(const FileSystemModel&);
ModelIndex index(int column) const;
void traverse_if_needed();
void reify_if_needed();
bool fetch_data(const String& full_path, bool is_root);
};

Expand Down

0 comments on commit f034932

Please sign in to comment.