Skip to content

Introduce Manager::addBooksFromDirectory() #1220

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Introduce Manager::addBooksFromDirectory() #1220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
46e7454
Introduce Manager::addBooksFromDirectory()
juuz0 Sep 6, 2025
a1f271b
fixup! Introduce Manager::addBooksFromDirectory()
juuz0 Sep 8, 2025
ed92c48
Remove skipInvalid flag
juuz0 Sep 8, 2025
2dc1989
fixup! fixup! Introduce Manager::addBooksFromDirectory()
juuz0 Sep 9, 2025
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
9 changes: 9 additions & 0 deletions include/manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,15 @@ class Manager
const std::string& url = "",
const bool checkMetaData = false);

/**
* Add all books from the directory tree into the library.
*
* @param path The path of the directory to scan.
* @param verboseFlag Verbose logs flag.
*/
void addBooksFromDirectory(const std::string& path,
const bool verboseFlag = false);

std::string writableLibraryPath;

bool m_hasSearchResult = false;
Expand Down
50 changes: 50 additions & 0 deletions src/manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@
#include "tools/pathTools.h"

#include <pugixml.hpp>
#include <filesystem>
#include <iostream>
#include <set>
#include <queue>

namespace fs = std::filesystem;

namespace kiwix
{
Expand Down Expand Up @@ -251,6 +257,50 @@ bool Manager::addBookFromPath(const std::string& pathToOpen,
.empty());
}

void Manager::addBooksFromDirectory(const std::string& path,
const bool verboseFlag)
{
std::set<std::string> iteratedDirs;
std::queue<std::string> dirQueue;
dirQueue.push(fs::absolute(path).u8string());
int totalBooksAdded = 0;
if (verboseFlag)
std::cout << "Adding books from the directory tree: " << dirQueue.front() << std::endl;

while (!dirQueue.empty()) {
const auto currentPath = dirQueue.front();
dirQueue.pop();
if (verboseFlag)
std::cout << "Visiting directory: " << currentPath << std::endl;
for (const auto& dirEntry : fs::directory_iterator(currentPath)) {
auto resolvedPath = dirEntry.path();
if (fs::is_symlink(dirEntry)) {
resolvedPath = fs::canonical(dirEntry.path());
}
const std::string pathString = resolvedPath.u8string();
if (fs::is_directory(resolvedPath)) {
if (iteratedDirs.find(pathString) == iteratedDirs.end())
dirQueue.push(pathString);
else if (verboseFlag)
std::cout << "Already iterated over " << pathString << ". Skipping..." << std::endl;
} else if (resolvedPath.extension() == ".zim" || resolvedPath.extension() == ".zimaa") {
if (!this->addBookFromPath(pathString, pathString, "", false)) {
Copy link
Collaborator

@veloman-yunkan veloman-yunkan Sep 8, 2025

Choose a reason for hiding this comment

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

  1. Is it wise trying to treat all files (regardless of their extension) as ZIM?
  2. Shouldn't this operation also be reported in the verbose output?

Copy link
Collaborator Author

@juuz0 juuz0 Sep 8, 2025

Choose a reason for hiding this comment

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

  1. Probably not, and with the recent changes in 1baa142. This would report a std:cerrr for each non ZIM file. I will push a commit for it but wanted to ask if you've tackled this before? Should checking the extensions for .zim, .zimaa, .zimab (and their capitals) be sufficient?

  2. Done in fixup.

Copy link
Collaborator

@veloman-yunkan veloman-yunkan Sep 8, 2025

Choose a reason for hiding this comment

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

Should checking the extensions for .zim, .zimaa, .zimab (and their capitals) be sufficient?

I think that you should go only after .zim and .zimaa files.

@kelson42 Please confirm

Copy link
Collaborator

@kelson42 kelson42 Sep 14, 2025

Choose a reason for hiding this comment

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

100% agree with @veloman-yunkan, this is already what I said in my first review, see kiwix/kiwix-tools#743 (comment)

std::cerr << "Could not add " << pathString << " into the library." << std::endl;
} else if (verboseFlag) {
std::cout << "Added " << pathString << " into the library." << std::endl;
totalBooksAdded++;
}
} else if (verboseFlag) {
std::cout << "Skipped " << pathString << " - unsupported file type or permission denied." << std::endl;
}
}
iteratedDirs.insert(currentPath);
}

if (verboseFlag)
std::cout << "Traversal completed. Total books added: " << totalBooksAdded << std::endl;
}

bool Manager::readBookFromPath(const std::string& path, kiwix::Book* book)
{
std::string tmp_path = path;
Expand Down
Loading