Respond to PR comments

src/logger/CMakeLists.txt

@@ -1,25 +1,19 @@
 set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 
-set(tgt acquire-zarr-logger)
-
-add_library(${tgt}
+add_library(acquire-logger
         logger.hh
         logger.cpp
 )
 
-set(PUBLIC_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/include/)
-
-target_include_directories(${tgt}
-        PUBLIC
-        $<BUILD_INTERFACE:${PUBLIC_INCLUDE_DIR}>
+target_include_directories(acquire-logger
         PRIVATE
         $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
 )
 
-set_target_properties(${tgt} PROPERTIES
+set_target_properties(acquire-logger PROPERTIES
         MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>"
 )
 
-install(TARGETS ${tgt}
+install(TARGETS acquire-logger
         LIBRARY DESTINATION lib
 )

src/logger/logger.cpp

@@ -2,83 +2,44 @@
 
 #include <cstdarg>
 #include <iomanip>
-#include <iostream>
-#include <filesystem>
 #include <string>
 #include <thread>
 
-ZarrLogLevel Logger::current_level_ = ZarrLogLevel_Info;
+LogLevel Logger::current_level_ = LogLevel_Info;
 std::mutex Logger::log_mutex_{};
 
 void
-Logger::set_log_level(ZarrLogLevel level)
+Logger::set_log_level(LogLevel level)
 {
     current_level_ = level;
 }
 
-ZarrLogLevel
+LogLevel
 Logger::get_log_level()
 {
     return current_level_;
 }
 
 std::string
-Logger::log(ZarrLogLevel level,
-            const char* file,
-            int line,
-            const char* func,
-            const char* format,
-            ...)
+Logger::get_timestamp_()
 {
-    std::scoped_lock lock(log_mutex_);
-    if (current_level_ == ZarrLogLevel_None || level < current_level_) {
-        return {}; // Suppress logs
-    }
 
-    va_list args;
-    va_start(args, format);
-
-    std::string prefix;
-    std::ostream* stream = &std::cout;
-
-    switch (level) {
-        case ZarrLogLevel_Debug:
-            prefix = "[DEBUG] ";
-            break;
-        case ZarrLogLevel_Info:
-            prefix = "[INFO] ";
-            break;
-        case ZarrLogLevel_Warning:
-            prefix = "[WARNING] ";
-            stream = &std::cerr;
-            break;
-        case ZarrLogLevel_Error:
-            prefix = "[ERROR] ";
-            stream = &std::cerr;
-            break;
-    }
-
-    // Get current time
     auto now = std::chrono::system_clock::now();
     auto time = std::chrono::system_clock::to_time_t(now);
     auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(
                 now.time_since_epoch()) %
               1000;
 
-    // Get filename without path
-    std::filesystem::path filepath(file);
-    std::string filename = filepath.filename().string();
-
-    // Output timestamp, log level, filename
-    *stream << std::put_time(std::localtime(&time), "%Y-%m-%d %H:%M:%S") << '.'
-            << std::setfill('0') << std::setw(3) << ms.count() << " " << prefix
-            << filename << ":" << line << " " << func << ": ";
-
-    char buffer[1024];
-    vsnprintf(buffer, sizeof(buffer), format, args);
-    *stream << buffer << std::endl;
+    std::tm tm{};
+#if defined(_WIN32)
+    localtime_s(&tm, &time);
+#else
+    localtime_r(&time, &tm_snapshot);
+#endif
 
-    va_end(args);
+    std::ostringstream ss;
+    ss << std::put_time(&tm, "%Y-%m-%d %H:%M:%S") << '.' << std::setfill('0')
+       << std::setw(3) << ms.count();
 
-    return buffer;
+    return ss.str();
 }

src/logger/logger.hh

@@ -1,30 +1,79 @@
-#include "zarr.types.h"
+#include "logger.types.h"
 
+#include <filesystem>
+#include <iostream>
 #include <mutex>
 
 class Logger
 {
   public:
-    static void set_log_level(ZarrLogLevel level);
-    static ZarrLogLevel get_log_level();
+    static void set_log_level(LogLevel level);
+    static LogLevel get_log_level();
 
-    static std::string log(ZarrLogLevel level,
+    template<typename... Args>
+    static std::string log(LogLevel level,
                            const char* file,
                            int line,
                            const char* func,
-                           const char* format,
-                           ...);
+                           Args&&... args)
+    {
+        namespace fs = std::filesystem;
+
+        std::scoped_lock lock(log_mutex_);
+
+        std::string prefix;
+        auto stream = &std::cout;
+
+        switch (level) {
+            case LogLevel_Debug:
+                prefix = "[DEBUG] ";
+                break;
+            case LogLevel_Info:
+                prefix = "[INFO] ";
+                break;
+            case LogLevel_Warning:
+                prefix = "[WARNING] ";
+                break;
+            default:
+                prefix = "[ERROR] ";
+                stream = &std::cerr;
+                break;
+        }
+
+        fs::path filepath(file);
+        std::string filename = filepath.filename().string();
+
+        std::ostringstream ss;
+        ss << get_timestamp_() << " " << prefix << filename << ":" << line
+           << " " << func << ": ";
+
+        format_arg_(ss, std::forward<Args>(args)...);
+
+        std::string message = ss.str();
+        *stream << message << std::endl;
+
+        return message;
+    }
 
   private:
-    static ZarrLogLevel current_level_;
+    static LogLevel current_level_;
     static std::mutex log_mutex_;
+
+    static void format_arg_(std::ostream& ss) {}; // base case
+    template<typename T, typename... Args>
+    static void format_arg_(std::ostream& ss, T&& arg, Args&&... args) {
+        ss << std::forward<T>(arg);
+        format_arg_(ss, std::forward<Args>(args)...);
+    }
+
+    static std::string get_timestamp_();
 };
 
 #define LOG_DEBUG(...)                                                         \
-      Logger::log(ZarrLogLevel_Debug, __FILE__, __LINE__, __func__, __VA_ARGS__)
+      Logger::log(LogLevel_Debug, __FILE__, __LINE__, __func__, __VA_ARGS__)
 #define LOG_INFO(...)                                                          \
     Logger::log(LogLevel_Info, __FILE__, __LINE__, __func__, __VA_ARGS__)
 #define LOG_WARNING(...)                                                       \
-    Logger::log(ZarrLogLevel_Warning, __FILE__, __LINE__, __func__, __VA_ARGS__)
+    Logger::log(LogLevel_Warning, __FILE__, __LINE__, __func__, __VA_ARGS__)
 #define LOG_ERROR(...)                                                         \
-    Logger::log(ZarrLogLevel_Error, __FILE__, __LINE__, __func__, __VA_ARGS__)
+    Logger::log(LogLevel_Error, __FILE__, __LINE__, __func__, __VA_ARGS__)

src/logger/logger.types.h

@@ -0,0 +1,8 @@
+#pragma once
+
+typedef enum {
+    LogLevel_Debug,
+    LogLevel_Info,
+    LogLevel_Warning,
+    LogLevel_Error
+} LogLevel;