From f695e536dd53bdf2ad10124a0206f2657d2055b2 Mon Sep 17 00:00:00 2001 From: gabime Date: Fri, 22 Dec 2017 18:37:51 +0200 Subject: [PATCH] Fixed file_helper::split_by_extenstion and added more tests for it --- include/spdlog/details/file_helper.h | 36 +++++---- tests/file_helper.cpp | 115 ++++++++------------------- 2 files changed, 55 insertions(+), 96 deletions(-) diff --git a/include/spdlog/details/file_helper.h b/include/spdlog/details/file_helper.h index 3b8df9c1..c131fc9b 100644 --- a/include/spdlog/details/file_helper.h +++ b/include/spdlog/details/file_helper.h @@ -109,27 +109,35 @@ public: } // - // return basename and extension: + // return file path and its extension: // // "mylog.txt" => ("mylog", ".txt") // "mylog" => ("mylog", "") + // "mylog." => ("mylog.", "") + // "/dir1/dir2/mylog.txt" => ("/dir1/dir2/mylog", ".txt") // // the starting dot in filenames is ignored (hidden files): // - // "my_folder/.mylog" => ("my_folder/.mylog") + // ".mylog" => (".mylog". "") + // "my_folder/.mylog" => ("my_folder/.mylog", "") // "my_folder/.mylog.txt" => ("my_folder/.mylog", ".txt") - static std::tuple split_by_extenstion(const filename_t& fname) - { - auto index = fname.rfind('.'); - if (index != filename_t::npos && index != fname.size() - 1 &&index !=0 && fname[index - 1] != details::os::folder_sep) - { - auto index2 = fname.find(details::os::folder_sep, index); - if (index2 == fname.npos) { - return std::make_tuple(fname.substr(0, index), fname.substr(index)); - } - } - return std::make_tuple(fname, std::string()); - } + static std::tuple split_by_extenstion(const spdlog::filename_t& fname) + { + auto ext_index = fname.rfind('.'); + + // no valid extension found - return whole path and empty string as extension + if (ext_index == filename_t::npos || ext_index == 0 || ext_index == fname.size() - 1) + return std::make_tuple(fname, spdlog::filename_t()); + + // treat casese like "/etc/rc.d/somelogfile or "/abc/.hiddenfile" + //auto folder_index = fname.find('\\', ext_index); + auto folder_index = fname.rfind(details::os::folder_sep); + if (folder_index != fname.npos && folder_index >= ext_index - 1) + return std::make_tuple(fname, spdlog::filename_t()); + + // finally - return a valid base and extnetion tuple + return std::make_tuple(fname.substr(0, ext_index), fname.substr(ext_index)); + } private: FILE* _fd; filename_t _filename; diff --git a/tests/file_helper.cpp b/tests/file_helper.cpp index 6c8ee79f..09688701 100644 --- a/tests/file_helper.cpp +++ b/tests/file_helper.cpp @@ -73,91 +73,42 @@ TEST_CASE("file_helper_reopen2", "[file_helper::reopen(false)]]") REQUIRE(helper.size() == expected_size); } + + +static void test_split_ext(spdlog::filename_t& filename, spdlog::filename_t & expected_base, spdlog::filename_t & expected_ext) +{ + +#ifdef _WIN32 // replace folder sep + std::replace(filename.begin(), filename.end(), '/', '\\'); + std::replace(expected_base.begin(), expected_base.end(), '/', '\\'); +#endif + spdlog::filename_t basename, ext; + std::tie(basename, ext) = file_helper::split_by_extenstion(filename); + REQUIRE(basename == expected_base); + REQUIRE(ext == expected_ext); +} + + TEST_CASE("file_helper_split_by_extenstion", "[file_helper::split_by_extenstion()]]") { - std::string basename, ext; - std::tie(basename, ext) = file_helper::split_by_extenstion("mylog.txt"); - REQUIRE(basename == "mylog"); - REQUIRE(ext == ".txt"); -} - -TEST_CASE("file_helper_split_by_extenstion2", "[file_helper::split_by_extenstion()]]") -{ - std::string basename, ext; - std::tie(basename, ext) = file_helper::split_by_extenstion("mylog"); - REQUIRE(basename == "mylog"); - REQUIRE(ext == ""); -} - -TEST_CASE("file_helper_split_by_extenstion3", "[file_helper::split_by_extenstion()]]") -{ - std::string basename, ext; - std::tie(basename, ext) = file_helper::split_by_extenstion("mylog.xyz.txt"); - REQUIRE(basename == "mylog.xyz"); - REQUIRE(ext == ".txt"); + + using file_t = spdlog::filename_t; + test_split_ext(file_t("mylog.txt"), file_t("mylog"), file_t(".txt")); + test_split_ext(file_t(".mylog.txt"), file_t(".mylog"), file_t(".txt")); + test_split_ext(file_t(".mylog"), file_t(".mylog"), file_t("")); + test_split_ext(file_t("/aaa/bb.d/mylog"), file_t("/aaa/bb.d/mylog"), file_t("")); + test_split_ext(file_t("/aaa/bb.d/mylog.txt"), file_t("/aaa/bb.d/mylog"), file_t(".txt")); + test_split_ext(file_t("aaa/bbb/ccc/mylog.txt"), file_t("aaa/bbb/ccc/mylog"), file_t(".txt")); + test_split_ext(file_t("aaa/bbb/ccc/mylog."), file_t("aaa/bbb/ccc/mylog."), file_t("")); + test_split_ext(file_t("aaa/bbb/ccc/.mylog.txt"), file_t("aaa/bbb/ccc/.mylog"), file_t(".txt")); + test_split_ext(file_t("/aaa/bbb/ccc/mylog.txt"), file_t("/aaa/bbb/ccc/mylog"), file_t(".txt")); + test_split_ext(file_t("/aaa/bbb/ccc/.mylog"), file_t("/aaa/bbb/ccc/.mylog"), file_t("")); + test_split_ext(file_t("../mylog.txt"), file_t("../mylog"), file_t(".txt")); + test_split_ext(file_t(".././mylog.txt"), file_t(".././mylog"), file_t(".txt")); + test_split_ext(file_t(".././mylog.txt/xxx"), file_t(".././mylog.txt/xxx"), file_t("")); + test_split_ext(file_t("/mylog.txt"), file_t("/mylog"), file_t(".txt")); + test_split_ext(file_t("//mylog.txt"), file_t("//mylog"), file_t(".txt")); } -TEST_CASE("file_helper_split_by_extenstion4", "[file_helper::split_by_extenstion()]]") -{ - std::string basename, ext; - std::tie(basename, ext) = file_helper::split_by_extenstion("mylog.xyz....txt"); - REQUIRE(basename == "mylog.xyz..."); - REQUIRE(ext == ".txt"); -} - -TEST_CASE("file_helper_split_by_extenstion5", "[file_helper::split_by_extenstion(hidden_file)]]") -{ - std::string basename, ext; - std::tie(basename, ext) = file_helper::split_by_extenstion(".mylog"); - REQUIRE(basename == ".mylog"); - REQUIRE(ext == ""); -} - -TEST_CASE("file_helper_split_by_extenstion6", "[file_helper::split_by_extenstion(hidden_file)]]") -{ -#ifdef _WIN32 - auto filename = "folder\\.mylog"; - auto expected_basename = "folder\\.mylog"; -#else - auto filename = "folder/.mylog"; - auto expected_basename = "folder/.mylog"; -#endif - std::string basename, ext; - std::tie(basename, ext) = file_helper::split_by_extenstion(filename); - REQUIRE(basename == expected_basename); - REQUIRE(ext == ""); -} - -TEST_CASE("file_helper_split_by_extenstion7", "[file_helper::split_by_extenstion(hidden_file)]]") -{ -#ifdef _WIN32 - auto filename = "folder\\.mylog.txt"; - auto expected_basename = "folder\\.mylog"; -#else - auto filename = "folder/.mylog.txt"; - auto expected_basename = "folder/.mylog"; -#endif - std::string basename, ext; - std::tie(basename, ext) = file_helper::split_by_extenstion(filename); - REQUIRE(basename == expected_basename); - REQUIRE(ext == ".txt"); -} - - -TEST_CASE("file_helper_split_by_extenstion8", "[file_helper::split_by_extenstion(hidden_file)]]") -{ -#ifdef _WIN32 - auto filename = "folder.ext\\mylog"; - auto expected_basename = "folder.ext\\mylog"; -#else - auto filename = "folder.ext/mylog"; - auto expected_basename = "folder.ext/mylog"; -#endif - std::string basename, ext; - std::tie(basename, ext) = file_helper::split_by_extenstion(filename); - REQUIRE(basename == expected_basename); - REQUIRE(ext == ""); -} -