From cbbb5c7fba604dca5907873e3c7767af8951470e Mon Sep 17 00:00:00 2001 From: Oleksii Gaidadim Date: Sat, 6 Jun 2026 18:35:58 +0200 Subject: [PATCH 1/6] Refactor: Improve directory iteration logging in AddCommand --- src/commands/AddCommand.cpp | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/commands/AddCommand.cpp b/src/commands/AddCommand.cpp index 4669811..26d5bf9 100644 --- a/src/commands/AddCommand.cpp +++ b/src/commands/AddCommand.cpp @@ -27,11 +27,22 @@ std::string DescribeSkippedEntry(const fs::path& path, const std::error_code& er std::string DescribeUnsupportedEntry(const fs::path& path) { return "Skipping unsupported entry: " + path.string(); } -void IncrementIterator(fs::recursive_directory_iterator& iterator, const fs::recursive_directory_iterator& end) { +std::string DescribeSymlinkEntry(const fs::directory_entry& entry) { + std::error_code errorCode; + const auto targetStatus = entry.status(errorCode); + if (!errorCode && fs::is_directory(targetStatus)) { + return "Skipping symlinked directory: " + entry.path().string(); + } + + return "Skipping symlink: " + entry.path().string(); +} + +void IncrementIterator(fs::recursive_directory_iterator& iterator, const fs::recursive_directory_iterator& end, + const fs::path& entryPath) { std::error_code errorCode; iterator.increment(errorCode); if (errorCode) { - utils::LogWarn("Skipping directory entry: " + errorCode.message()); + utils::LogWarn(DescribeSkippedEntry(entryPath, errorCode)); iterator = end; } } @@ -53,30 +64,30 @@ std::vector CollectOrdinaryFiles(const fs::path& directoryPath) { if (errorCode) { iterator.disable_recursion_pending(); utils::LogWarn(DescribeSkippedEntry(entryPath, errorCode)); - IncrementIterator(iterator, end); + IncrementIterator(iterator, end, entryPath); continue; } if (fs::is_symlink(status)) { iterator.disable_recursion_pending(); - utils::LogWarn("Skipping symlink: " + entryPath.string()); - IncrementIterator(iterator, end); + utils::LogWarn(DescribeSymlinkEntry(*iterator)); + IncrementIterator(iterator, end, entryPath); continue; } if (fs::is_directory(status)) { - IncrementIterator(iterator, end); + IncrementIterator(iterator, end, entryPath); continue; } if (fs::is_regular_file(status)) { files.push_back(utils::NormalizePath(entryPath)); - IncrementIterator(iterator, end); + IncrementIterator(iterator, end, entryPath); continue; } utils::LogWarn(DescribeUnsupportedEntry(entryPath)); - IncrementIterator(iterator, end); + IncrementIterator(iterator, end, entryPath); } std::sort(files.begin(), files.end(), [](const fs::path& left, const fs::path& right) { From 458916d347bafea7329b02ac455adcf56783ff59 Mon Sep 17 00:00:00 2001 From: Oleksii Gaidadim Date: Sat, 6 Jun 2026 18:35:59 +0200 Subject: [PATCH 2/6] Test: Add tests for AddCommand directory iteration skipping behavior --- tests/AddCommandTests.cpp | 48 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/AddCommandTests.cpp b/tests/AddCommandTests.cpp index ce14fd3..bae7c69 100644 --- a/tests/AddCommandTests.cpp +++ b/tests/AddCommandTests.cpp @@ -11,6 +11,10 @@ #include #include +#ifndef _WIN32 +#include +#endif + namespace { namespace fs = std::filesystem; @@ -134,6 +138,29 @@ TEST_F(AddCommandTest, DirectoryImportSkipsSymlinksAndKeepsImportingOrdinaryFile EXPECT_EQ(Registry().GetTrackedEntries()[0].OriginalPath, cfgsync::utils::NormalizePath(sourcePath).string()); } +TEST_F(AddCommandTest, DirectoryImportSkipsSymlinkedDirectoriesWithoutImportingTheirContents) { + const auto directoryPath = SourcePath().parent_path(); + const auto sourcePath = directoryPath / ".gitconfig"; + const auto linkedDirectoryTarget = StorageRoot() / "external-configs"; + const auto linkedFilePath = linkedDirectoryTarget / "secret.conf"; + const auto symlinkPath = directoryPath / "linked-configs"; + WriteTextFile(sourcePath, "[user]\n"); + WriteTextFile(linkedFilePath, "secret\n"); + + std::error_code errorCode; + fs::create_directory_symlink(linkedDirectoryTarget, symlinkPath, errorCode); + if (errorCode) { + GTEST_SKIP() << "Directory symlink creation is not available in this test environment."; + } + + cfgsync::commands::AddCommand command{Registry()}; + + command.Execute(directoryPath); + + ASSERT_EQ(Registry().GetTrackedEntries().size(), 1U); + EXPECT_EQ(Registry().GetTrackedEntries()[0].OriginalPath, cfgsync::utils::NormalizePath(sourcePath).string()); +} + TEST_F(AddCommandTest, EmptyDirectoryImportSucceedsWithoutChangingRegistry) { const auto directoryPath = SourcePath().parent_path(); cfgsync::utils::EnsureDirectoryExists(directoryPath); @@ -183,6 +210,27 @@ TEST_F(AddCommandTest, DirectoryImportContinuesAfterUnreadableSubdirectoryWhenPe } #endif +#ifndef _WIN32 +TEST_F(AddCommandTest, DirectoryImportSkipsSpecialFilesAndKeepsImportingOrdinaryFiles) { + const auto directoryPath = SourcePath().parent_path(); + const auto sourcePath = directoryPath / ".gitconfig"; + const auto fifoPath = directoryPath / "events.fifo"; + WriteTextFile(sourcePath, "[user]\n"); + + const auto result = mkfifo(fifoPath.c_str(), S_IRUSR | S_IWUSR); + if (result != 0) { + GTEST_SKIP() << "Unable to create a FIFO in this test environment."; + } + + cfgsync::commands::AddCommand command{Registry()}; + + command.Execute(directoryPath); + + ASSERT_EQ(Registry().GetTrackedEntries().size(), 1U); + EXPECT_EQ(Registry().GetTrackedEntries()[0].OriginalPath, cfgsync::utils::NormalizePath(sourcePath).string()); +} +#endif + TEST_F(AddCommandTest, MissingFileFailsClearly) { cfgsync::utils::EnsureDirectoryExists(SourcePath().parent_path()); cfgsync::commands::AddCommand command{Registry()}; From c6e4ae98923ddc15d35564d0178ebab14684d83a Mon Sep 17 00:00:00 2001 From: Oleksii Gaidadim Date: Sat, 6 Jun 2026 18:35:59 +0200 Subject: [PATCH 3/6] Feat: Report skipped already-tracked files during directory add --- src/commands/AddCommand.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/commands/AddCommand.cpp b/src/commands/AddCommand.cpp index 26d5bf9..95ff6d5 100644 --- a/src/commands/AddCommand.cpp +++ b/src/commands/AddCommand.cpp @@ -137,9 +137,12 @@ void AddCommand::ExecuteDirectory(const fs::path& normalizedPath) const { } auto addedCount = 0U; + auto duplicateCount = 0U; for (const auto& filePath : files) { if (AddFileEntry(filePath)) { ++addedCount; + } else { + ++duplicateCount; } } @@ -149,7 +152,11 @@ void AddCommand::ExecuteDirectory(const fs::path& normalizedPath) const { } Registry_.Save(); - utils::LogInfo(std::format("Imported {} file(s) from directory: {}", addedCount, normalizedPath.string())); + auto summary = std::format("Imported {} file(s) from directory: {}", addedCount, normalizedPath.string()); + if (duplicateCount > 0U) { + summary += std::format(" (skipped {} already tracked file(s))", duplicateCount); + } + utils::LogInfo(summary); } void AddCommand::Execute(const fs::path& path) const { From 50dd8e7d77b2a1cf9707697464cd5d6f495765b4 Mon Sep 17 00:00:00 2001 From: Oleksii Gaidadim Date: Sat, 6 Jun 2026 18:35:59 +0200 Subject: [PATCH 4/6] Test: Enhance CLI tests for AddCommand output messages --- tests/AddCommandCliTests.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/AddCommandCliTests.cpp b/tests/AddCommandCliTests.cpp index 965602c..bb88255 100644 --- a/tests/AddCommandCliTests.cpp +++ b/tests/AddCommandCliTests.cpp @@ -78,6 +78,8 @@ TEST_F(AddCommandCliTest, AddDirectoryUsesActiveStorageRootAndWritesImportedFile const auto result = RunCommand("add " + cfgsync::tests::QuoteForCommand(directoryPath)); EXPECT_EQ(result.ExitCode, 0); + EXPECT_NE(result.Output.find("Imported 2 file(s) from directory"), std::string::npos); + EXPECT_NE(result.Output.find(directoryPath.string()), std::string::npos); EXPECT_TRUE(result.Error.empty()); const std::vector expectedPaths{ @@ -109,10 +111,37 @@ TEST_F(AddCommandCliTest, RepeatedDirectoryAddSucceedsAndLeavesRegistryUnchanged const auto result = RunCommand("add " + cfgsync::tests::QuoteForCommand(directoryPath)); EXPECT_EQ(result.ExitCode, 0); + EXPECT_NE(result.Output.find("No new files to track under directory"), std::string::npos); + EXPECT_NE(result.Output.find(directoryPath.string()), std::string::npos); EXPECT_TRUE(result.Error.empty()); EXPECT_EQ(ReadJsonFile(StorageRoot() / "registry.json"), documentAfterFirstAdd); } +TEST_F(AddCommandCliTest, DirectoryAddReportsSkippedAlreadyTrackedFilesWhenImportingNewFiles) { + const auto directoryPath = GetTestRoot() / "configs"; + const auto existingPath = directoryPath / ".gitconfig"; + const auto newPath = directoryPath / "nvim" / "init.lua"; + WriteTextFile(existingPath, "[user]\n"); + WriteTextFile(newPath, "vim.opt.number = true\n"); + + ASSERT_TRUE(cfgsync::tests::CfgsyncCommandSucceeded( + "init --storage " + cfgsync::tests::QuoteForCommand(StorageRoot()), GetTestRoot())); + ASSERT_TRUE( + cfgsync::tests::CfgsyncCommandSucceeded("add " + cfgsync::tests::QuoteForCommand(existingPath), GetTestRoot())); + + const auto result = RunCommand("add " + cfgsync::tests::QuoteForCommand(directoryPath)); + + EXPECT_EQ(result.ExitCode, 0); + EXPECT_NE(result.Output.find("Imported 1 file(s) from directory"), std::string::npos); + EXPECT_NE(result.Output.find("skipped 1 already tracked file(s)"), std::string::npos); + EXPECT_TRUE(result.Error.empty()); + + const auto document = ReadJsonFile(StorageRoot() / "registry.json"); + ASSERT_EQ(document["tracked_files"].size(), 2U); + EXPECT_EQ(document["tracked_files"][0]["original_path"], cfgsync::utils::NormalizePath(existingPath).string()); + EXPECT_EQ(document["tracked_files"][1]["original_path"], cfgsync::utils::NormalizePath(newPath).string()); +} + } // namespace int main(int argc, char** argv) { From a7407d0d4ab83718d84df1ebf48b54b1477128e3 Mon Sep 17 00:00:00 2001 From: Oleksii Gaidadim Date: Sat, 6 Jun 2026 19:01:19 +0200 Subject: [PATCH 5/6] refactor(add): Modernize AddCommand with C++20 ranges and if-initializers --- src/commands/AddCommand.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/commands/AddCommand.cpp b/src/commands/AddCommand.cpp index 95ff6d5..44fa4ad 100644 --- a/src/commands/AddCommand.cpp +++ b/src/commands/AddCommand.cpp @@ -29,8 +29,7 @@ std::string DescribeUnsupportedEntry(const fs::path& path) { return "Skipping un std::string DescribeSymlinkEntry(const fs::directory_entry& entry) { std::error_code errorCode; - const auto targetStatus = entry.status(errorCode); - if (!errorCode && fs::is_directory(targetStatus)) { + if (const auto targetStatus = entry.status(errorCode); !errorCode && fs::is_directory(targetStatus)) { return "Skipping symlinked directory: " + entry.path().string(); } @@ -90,7 +89,7 @@ std::vector CollectOrdinaryFiles(const fs::path& directoryPath) { IncrementIterator(iterator, end, entryPath); } - std::sort(files.begin(), files.end(), [](const fs::path& left, const fs::path& right) { + std::ranges::sort(files, [](const fs::path& left, const fs::path& right) { return left.generic_string() < right.generic_string(); }); From b32238fe329cd5b686083c0a4529823a11e8461d Mon Sep 17 00:00:00 2001 From: Oleksii Gaidadim Date: Sat, 6 Jun 2026 19:01:19 +0200 Subject: [PATCH 6/6] feat(add): Adopt custom CommandError for AddCommand exceptions --- src/commands/AddCommand.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/commands/AddCommand.cpp b/src/commands/AddCommand.cpp index 44fa4ad..1f26117 100644 --- a/src/commands/AddCommand.cpp +++ b/src/commands/AddCommand.cpp @@ -1,12 +1,12 @@ #include "commands/AddCommand.hpp" +#include "Exceptions.hpp" #include "utils/FileUtils.hpp" #include "utils/LogUtils.hpp" #include "utils/PathUtils.hpp" #include #include -#include #include #include @@ -103,15 +103,14 @@ AddCommand::AddCommand(core::Registry& registry) : Registry_(registry) {} bool AddCommand::AddFileEntry(const fs::path& normalizedPath) const { const auto storedRelativePath = utils::MakeStorageRelativePath(normalizedPath); if (storedRelativePath.empty()) { - throw std::logic_error{"Unable to derive a storage path for: " + normalizedPath.string()}; + throw CommandError{"Unable to derive a storage path for: " + normalizedPath.string()}; } - const auto added = Registry_.AddEntry({ - .OriginalPath = normalizedPath.string(), - .StoredRelativePath = storedRelativePath.generic_string(), - }); - - if (!added) { + if (const auto added = Registry_.AddEntry({ + .OriginalPath = normalizedPath.string(), + .StoredRelativePath = storedRelativePath.generic_string(), + }); + !added) { utils::LogInfo("File is already tracked: " + normalizedPath.string()); return false; }