From 8bc619bda9c03e66fec5c7bef803e393662152be Mon Sep 17 00:00:00 2001 From: Niels Dekker Date: Thu, 9 Feb 2023 15:54:59 +0100 Subject: [PATCH] Allow basic_file_sink to open a file in text mode Added an optional `bool` parameter to the explicit `basic_file_sink` constructor, allowing to open the file in in text mode, instead of in binary mode. Enables automatic conversion of '\n' characters _inside_ a message string to a platform specific eol representation. (`\r\n` on Windows.) --- include/spdlog/details/file_helper-inl.h | 10 +++++----- include/spdlog/details/file_helper.h | 2 +- include/spdlog/details/os-inl.h | 2 +- include/spdlog/sinks/base_sink-inl.h | 11 ++++++++++- include/spdlog/sinks/basic_file_sink-inl.h | 9 ++++++--- include/spdlog/sinks/basic_file_sink.h | 3 ++- tests/test_file_logging.cpp | 22 ++++++++++++++++++++++ 7 files changed, 47 insertions(+), 12 deletions(-) diff --git a/include/spdlog/details/file_helper-inl.h b/include/spdlog/details/file_helper-inl.h index 3c45d8c0..895ffb8f 100644 --- a/include/spdlog/details/file_helper-inl.h +++ b/include/spdlog/details/file_helper-inl.h @@ -29,13 +29,13 @@ SPDLOG_INLINE file_helper::~file_helper() close(); } -SPDLOG_INLINE void file_helper::open(const filename_t &fname, bool truncate) +SPDLOG_INLINE void file_helper::open(const filename_t &fname, bool truncate, bool text_mode) { close(); filename_ = fname; - auto *mode = SPDLOG_FILENAME_T("ab"); - auto *trunc_mode = SPDLOG_FILENAME_T("wb"); + auto *mode = text_mode ? SPDLOG_FILENAME_T("a") : SPDLOG_FILENAME_T("ab"); + auto *trunc_mode = text_mode ? SPDLOG_FILENAME_T("w") : SPDLOG_FILENAME_T("wb"); if (event_handlers_.before_open) { @@ -47,8 +47,8 @@ SPDLOG_INLINE void file_helper::open(const filename_t &fname, bool truncate) os::create_dir(os::dir_name(fname)); if (truncate) { - // Truncate by opening-and-closing a tmp file in "wb" mode, always - // opening the actual log-we-write-to in "ab" mode, since that + // Truncate by opening-and-closing a tmp file in trunc mode, always + // opening the actual log-we-write-to in append mode, since that // interacts more politely with eternal processes that might // rotate/truncate the file underneath us. std::FILE *tmp; diff --git a/include/spdlog/details/file_helper.h b/include/spdlog/details/file_helper.h index f42a5eb1..a45fe05f 100644 --- a/include/spdlog/details/file_helper.h +++ b/include/spdlog/details/file_helper.h @@ -23,7 +23,7 @@ public: file_helper &operator=(const file_helper &) = delete; ~file_helper(); - void open(const filename_t &fname, bool truncate = false); + void open(const filename_t &fname, bool truncate = false, bool text_mode = false); void reopen(bool truncate); void flush(); void sync(); diff --git a/include/spdlog/details/os-inl.h b/include/spdlog/details/os-inl.h index 19d4bdc5..8f89b116 100644 --- a/include/spdlog/details/os-inl.h +++ b/include/spdlog/details/os-inl.h @@ -142,7 +142,7 @@ SPDLOG_INLINE bool fopen_s(FILE **fp, const filename_t &filename, const filename # endif #else // unix # if defined(SPDLOG_PREVENT_CHILD_FD) - const int mode_flag = mode == SPDLOG_FILENAME_T("ab") ? O_APPEND : O_TRUNC; + const int mode_flag = (mode.find('a') == filename_t::npos) ? O_TRUNC : O_APPEND; const int fd = ::open((filename.c_str()), O_CREAT | O_WRONLY | O_CLOEXEC | mode_flag, mode_t(0644)); if (fd == -1) { diff --git a/include/spdlog/sinks/base_sink-inl.h b/include/spdlog/sinks/base_sink-inl.h index 421fdf9d..972278eb 100644 --- a/include/spdlog/sinks/base_sink-inl.h +++ b/include/spdlog/sinks/base_sink-inl.h @@ -53,7 +53,16 @@ void SPDLOG_INLINE spdlog::sinks::base_sink::set_formatter(std::unique_pt template void SPDLOG_INLINE spdlog::sinks::base_sink::set_pattern_(const std::string &pattern) { - set_formatter_(details::make_unique(pattern)); + const auto current_pattern_formatter = dynamic_cast(formatter_.get()); + + if (current_pattern_formatter) + { + current_pattern_formatter->set_pattern(pattern); + } + else + { + set_formatter_(details::make_unique(pattern)); + } } template diff --git a/include/spdlog/sinks/basic_file_sink-inl.h b/include/spdlog/sinks/basic_file_sink-inl.h index 8d23f96d..41da303d 100644 --- a/include/spdlog/sinks/basic_file_sink-inl.h +++ b/include/spdlog/sinks/basic_file_sink-inl.h @@ -9,15 +9,18 @@ #include #include +#include namespace spdlog { namespace sinks { template -SPDLOG_INLINE basic_file_sink::basic_file_sink(const filename_t &filename, bool truncate, const file_event_handlers &event_handlers) - : file_helper_{event_handlers} +SPDLOG_INLINE basic_file_sink::basic_file_sink( + const filename_t &filename, bool truncate, const file_event_handlers &event_handlers, bool text_mode) + : base_sink{details::make_unique(pattern_time_type::local, text_mode ? "\n" : details::os::default_eol)} + , file_helper_{event_handlers} { - file_helper_.open(filename, truncate); + file_helper_.open(filename, truncate, text_mode); } template diff --git a/include/spdlog/sinks/basic_file_sink.h b/include/spdlog/sinks/basic_file_sink.h index aacc993b..f788002a 100644 --- a/include/spdlog/sinks/basic_file_sink.h +++ b/include/spdlog/sinks/basic_file_sink.h @@ -20,7 +20,8 @@ template class basic_file_sink final : public base_sink { public: - explicit basic_file_sink(const filename_t &filename, bool truncate = false, const file_event_handlers &event_handlers = {}); + explicit basic_file_sink( + const filename_t &filename, bool truncate = false, const file_event_handlers &event_handlers = {}, bool text_mode = false); const filename_t &filename() const; protected: diff --git a/tests/test_file_logging.cpp b/tests/test_file_logging.cpp index 1c7a1853..0ab66c8e 100644 --- a/tests/test_file_logging.cpp +++ b/tests/test_file_logging.cpp @@ -23,6 +23,28 @@ TEST_CASE("simple_file_logger", "[simple_logger]]") REQUIRE(file_contents(SIMPLE_LOG) == spdlog::fmt_lib::format("Test message 1{}Test message 2{}", default_eol, default_eol)); } +TEST_CASE("text_file_logger", "[text_file_logger]]") +{ + spdlog::filename_t filename = SPDLOG_FILENAME_T(SIMPLE_LOG); + + for (const bool text_mode : {false, true}) + { + prepare_logdir(); + + auto sink = std::make_shared(filename, false, spdlog::file_event_handlers{}, text_mode); + sink->set_pattern("%v"); + spdlog::logger logger("logger", sink); + + logger.info("Test line {}\nTest line {}", 1, 2); + + logger.flush(); + require_message_count(SIMPLE_LOG, 2); + using spdlog::details::os::default_eol; + REQUIRE(file_contents(SIMPLE_LOG) == + spdlog::fmt_lib::format("Test line 1{}Test line 2{}", text_mode ? default_eol : "\n", default_eol)); + } +} + TEST_CASE("flush_on", "[flush_on]]") { prepare_logdir();