Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add list_files filtering #106

Merged
merged 2 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion base/fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,20 @@ namespace base {
// elements (is a normalized path).
std::string get_absolute_path(const std::string& path);

paths list_files(const std::string& path);
// Item types that list_files() can be filtered by
enum class ItemType {
All,
Directories,
Files
};
Comment on lines +68 to +73
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if using flags might simplify the code (minor change: fixed the indentation):

Suggested change
// Item types that list_files() can be filtered by
enum class ItemType {
All,
Directories,
Files
};
// Item types that list_files() can be filtered by
enum class ItemType {
Directories = 1,
Files = 2,
All = 3
};
LAF_ENUM_FLAGS(ItemType);

The LAF_ENUM_FLAGS macro is something new to make flags easier to use for enum class-type (you will need to rebase to the latest main to use this macro if you wish).

Anyway this change is not needed, we can use the ItemType as you've defined it, just fixing the indentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll keep a simple enum class since you can't really mix and match flags in this case, if you want Directories | Files you use "All", and if you want Directories | All... you... well you shouldn't want that, that makes no sense 🤪


// List all the items in a given path by default, two other parameters second parameters:
// filter can be use to distinguish between All items, directories and files.
// The name search can be used to match files by extension with something like "*.png" or by exact
// match without wildcards.
paths list_files(const std::string& path,
ItemType filter = ItemType::All,
const std::string& = "*");

// Returns true if the given character is a valud path separator
// (any of '\' or '/' characters).
Expand Down
59 changes: 59 additions & 0 deletions base/fs_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,65 @@ TEST(FS, CopyFiles)
EXPECT_EQ(data, read_file_content(dst));
}

TEST(FS, ListFiles)
{
// Prepare files
make_directory("a");
EXPECT_TRUE(is_directory("a"));

make_directory("a/b");
EXPECT_TRUE(is_directory("a/b"));
make_directory("a/c");
EXPECT_TRUE(is_directory("a/b"));

write_file_content("a/d", (uint8_t*)"123", 3);
EXPECT_TRUE(is_file("a/d"));

write_file_content("a/e", (uint8_t*)"321", 3);
EXPECT_TRUE(is_file("a/e"));

// Test normal find with asterisk match
EXPECT_TRUE(list_files("non-existent-folder").empty());
EXPECT_EQ(list_files("a").size(), 4);

auto dirs = list_files("a", ItemType::Directories);
EXPECT_EQ(dirs.size(), 2);
EXPECT_TRUE(std::find(dirs.begin(), dirs.end(), "b") != dirs.end());
EXPECT_TRUE(std::find(dirs.begin(), dirs.end(), "c") != dirs.end());

auto files = list_files("a", ItemType::Files);
EXPECT_EQ(files.size(), 2);
EXPECT_TRUE(std::find(files.begin(), files.end(), "d") != files.end());
EXPECT_TRUE(std::find(files.begin(), files.end(), "e") != files.end());

// Test pattern matching
make_directory("a/c-match-me");
EXPECT_TRUE(is_directory("a/c-match-me"));

EXPECT_EQ(list_files("a", ItemType::Directories, "c-*-me").size(), 1);
EXPECT_EQ(list_files("a", ItemType::Directories, "c-match-me").size(), 1);
EXPECT_EQ(list_files("a", ItemType::Files, "c-*-me").size(), 0);

write_file_content("a/c-alsomatch-me", (uint8_t*)"321", 3);
EXPECT_TRUE(is_file("a/c-alsomatch-me"));

EXPECT_EQ(list_files("a", ItemType::Files, "c-*-me").size(), 1);
EXPECT_EQ(list_files("a", ItemType::Files, "c-alsomatch-me").size(), 1);
EXPECT_EQ(list_files("a", ItemType::All, "c-*").size(), 2);
EXPECT_EQ(list_files("a", ItemType::All, "*-me").size(), 2);
EXPECT_EQ(list_files("a", ItemType::All, "*match*").size(), 2);
EXPECT_EQ(list_files("a", ItemType::All).size(), 6);

// Remove files
delete_file("a/e");
delete_file("a/d");
delete_file("a/c-alsomatch-me");
remove_directory("a/c-match-me");
remove_directory("a/c");
remove_directory("a/b");
remove_directory("a");
}

int main(int argc, char** argv)
{
::testing::InitGoogleTest(&argc, argv);
Expand Down
29 changes: 21 additions & 8 deletions base/fs_unix.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <fnmatch.h>

#include <cerrno>
#include <climits>
Expand Down Expand Up @@ -229,20 +230,32 @@ std::string get_absolute_path(const std::string& path)
return full;
}

paths list_files(const std::string& path)
paths list_files(const std::string& path, ItemType filter, const std::string& match)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'list_files' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers]

paths list_files(const std::string& path, ItemType filter, const std::string& match)
      ^
Additional context

base/fs_unix.h:232: make as 'inline'

paths list_files(const std::string& path, ItemType filter, const std::string& match)
      ^

{
paths files;
DIR* handle = opendir(path.c_str());
if (handle) {
dirent* item;
while ((item = readdir(handle)) != nullptr) {
std::string filename = item->d_name;
if (filename != "." && filename != "..")
files.push_back(filename);
if (!handle)
return files;

dirent* item;
while ((item = readdir(handle)) != nullptr) {
ckaiser marked this conversation as resolved.
Show resolved Hide resolved
if (item->d_type == DT_DIR) {
if (filter == ItemType::Files)
continue;

if (strcmp(item->d_name, ".") == 0 || strcmp(item->d_name, "..") == 0)
continue;
}
else if (filter == ItemType::Directories)
continue;

closedir(handle);
if (fnmatch(match.c_str(), item->d_name, FNM_CASEFOLD) == FNM_NOMATCH)
continue;

files.push_back(item->d_name);
}

closedir(handle);
return files;
}

Expand Down
41 changes: 31 additions & 10 deletions base/fs_win32.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,19 +180,40 @@ std::string get_absolute_path(const std::string& path)
return to_utf8(buffer);
}

paths list_files(const std::string& path)
paths list_files(const std::string& path,
ItemType filter,
const std::string& match)
{
WIN32_FIND_DATA fd;
paths files;
HANDLE handle = FindFirstFile(base::from_utf8(base::join_path(path, "*")).c_str(), &fd);
if (handle) {
do {
std::string filename = base::to_utf8(fd.cFileName);
if (filename != "." && filename != "..")
files.push_back(filename);
} while (FindNextFile(handle, &fd));
FindClose(handle);
}
HANDLE handle = FindFirstFileEx(
base::from_utf8(base::join_path(path, match)).c_str(),
FindExInfoBasic,
&fd,
(filter == ItemType::Directories) ? FindExSearchLimitToDirectories :
FindExSearchNameMatch,
NULL,
0);

if (handle == INVALID_HANDLE_VALUE)
return files;

do {
if (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
if (filter == ItemType::Files)
continue;

if (lstrcmpW(fd.cFileName, L".") == 0 ||
lstrcmpW(fd.cFileName, L"..") == 0)
continue;
}
else if (filter == ItemType::Directories)
continue;

files.push_back(base::to_utf8(fd.cFileName));
} while (FindNextFile(handle, &fd));

FindClose(handle);
return files;
}

Expand Down
Loading