Merge pull request #7 from StrataSource/refactor/simplify-logging

Simplify the logging system

.gitignore

@@ -1,3 +1,4 @@
 run
 .idea
-cmake-build-*
+cmake-build-*
+build/

CMakeLists.txt

@@ -30,20 +30,12 @@ endif()
 # Add sources for CLI executable
 list( APPEND ${PROJECT_NAME}_SOURCES
 	"${CMAKE_CURRENT_LIST_DIR}/src/main.cpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/Output.cpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/Output.hpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/CsvOutput.cpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/CsvOutput.hpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/JsonOutput.cpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/JsonOutput.hpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/SimpleOutput.cpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/SimpleOutput.hpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/RsvOutput.cpp"
-	"${CMAKE_CURRENT_LIST_DIR}/src/output/RsvOutput.hpp"
 	"${CMAKE_CURRENT_LIST_DIR}/src/create.cpp"
 	"${CMAKE_CURRENT_LIST_DIR}/src/create.hpp"
 	"${CMAKE_CURRENT_LIST_DIR}/src/verify.cpp"
 	"${CMAKE_CURRENT_LIST_DIR}/src/verify.hpp"
+	"${CMAKE_CURRENT_LIST_DIR}/src/log.cpp"
+	"${CMAKE_CURRENT_LIST_DIR}/src/log.hpp"
 )
 
 # Create CLI executable

src/create.cpp

@@ -2,8 +2,6 @@
 // Created by ENDERZOMBI102 on 15/10/2023.
 //
 #include <filesystem>
-#include <fmt/chrono.h>
-#include <fmt/format.h>
 #include <fstream>
 #include <hash-library/crc32.h>
 #include <hash-library/sha256.h>
@@ -12,44 +10,45 @@
 #include <string_view>
 
 #include "create.hpp"
+#include "log.hpp"
 
 
-auto create( std::string_view root_, std::string_view indexLocation, const std::vector<std::string>& excluded, bool overwrite, const Output* out ) noexcept -> int {
+auto create( std::string_view root_, std::string_view indexLocation, const std::vector<std::string>& excluded, bool overwrite ) noexcept -> int {
 	const std::filesystem::path root{ root_ };
 	const std::filesystem::path indexPath{ root / indexLocation };
 
 	if ( std::filesystem::exists( indexPath ) ) {
 		if (! overwrite ) {
-			out->write( OutputKind::Error, fmt::format( "Index file `{}` already exist, do you want to overwite it? (y/N)", indexPath.string() ) );
+			Log_Error( "Index file `{}` already exist, do you want to overwite it? (y/N)", indexPath.string() );
 			std::string input;
 			std::cin >> input;
 			if ( input != "y" ) {
-				out->write( OutputKind::Info, "Aborting." );
+				Log_Info( "Aborting." );
 				return 1;
 			}
 		} else {
-			out->write( OutputKind::Warn, fmt::format( "Index file `{}` already exist, will be overwritten.", indexPath.string() ) );
+			Log_Warn( "Index file `{}` already exist, will be overwritten.", indexPath.string() );
 		}
 	}
 
 	auto start{ std::chrono::high_resolution_clock::now() };
-	out->write( OutputKind::Info, fmt::format( "Creating index file at `{}`", indexPath.string() ) );
+	Log_Info( "Creating index file at `{}`", indexPath.string() );
 
 	// open index file with a writer stream
 	std::ofstream writer{ indexPath, std::ios_base::out | std::ios_base::trunc };
 	if (! writer.good() ) {
-		out->write( OutputKind::Error, fmt::format( "Failed to open index file for writing: N/D" ) );
+		Log_Error( "Failed to open index file for writing: N/D" );
 		return 1;
 	}
 
-	out->write( OutputKind::Info, "Compiling exclusion regexes..." );
+	Log_Info( "Compiling exclusion regexes..." );
 	// allocate all at once
 	std::vector<std::regex> exclusionREs{};
 	exclusionREs.reserve( excluded.size() );
 	for ( const auto& exclusion : excluded ) {
 		exclusionREs.emplace_back( exclusion, std::regex::ECMAScript | std::regex::icase | std::regex::optimize );
 	}
-	out->write( OutputKind::Info, fmt::format( "Done in {}", std::chrono::duration_cast<std::chrono::milliseconds>( std::chrono::high_resolution_clock::now() - start ) ) );
+	Log_Info( "Done in {}", std::chrono::duration_cast<std::chrono::milliseconds>( std::chrono::high_resolution_clock::now() - start ) );
 
 	unsigned count{ 0 };
 	unsigned errors{ 0 };
@@ -83,7 +82,7 @@ auto create( std::string_view root_, std::string_view indexLocation, const std::
 		fopen_s( &file, path.c_str(), "rb" );
 #endif
 		if (! file ) {
-			out->write( OutputKind::Error, fmt::format( "Failed to open file: {}", path ) );
+			Log_Error( "Failed to open file: {}", path );
 			continue;
 		}
 
@@ -106,20 +105,12 @@ auto create( std::string_view root_, std::string_view indexLocation, const std::
 
 		// write out entry
 		writer << fmt::format( "{}\xFF{}\xFF{}\xFF{}\xFF\xFD", pathRel, size, sha256er.getHash(), crc32er.getHash() );
-		out->write( OutputKind::Info, fmt::format( "Processed file `{}`", path ) );
+		Log_Info( "Processed file `{}`", path );
 		count += 1;
 	}
 
 	auto end{ std::chrono::high_resolution_clock::now() };
-	out->write(
-		OutputKind::Info,
-		fmt::format(
-			"Finished processing {} files in {}! (with {} errors)",
-			count,
-			std::chrono::duration_cast<std::chrono::seconds>( end - start ),
-			errors
-		)
-	);
+	Log_Info( "Finished processing {} files in {}! (with {} errors)", count, std::chrono::duration_cast<std::chrono::seconds>( end - start ), errors );
 
 	return 0;
 }

src/create.hpp

@@ -6,7 +6,4 @@
 #include <vector>
 #include <string_view>
 
-#include "output/Output.hpp"
-
-
-auto create( std::string_view root, std::string_view indexLocation, const std::vector<std::string>& excluded, bool overwrite, const Output* out ) noexcept -> int;
+auto create( std::string_view root, std::string_view indexLocation, const std::vector<std::string>& excluded, bool overwrite ) noexcept -> int;

src/log.cpp

@@ -0,0 +1,41 @@
+#include "log.hpp"
+#include <iostream>
+
+bool g_bUIReportMode = false;
+
+void Log_Message( LogSeverity severity, const std::string_view message )
+{
+	// Don't log in report only mode!
+	const char* prefixes[] = {
+		"Info",
+		"Warn",
+		"Error",
+	};
+	const char* prefix = prefixes[(int)severity];
+
+	if ( g_bUIReportMode )
+	{
+		// Print for UI to read
+		const auto line{ fmt::format( R"("message","{}","{}",,)", prefix, message ) };
+		std::printf( "%s\n", line.c_str() );
+	}
+	else
+	{
+		// Standard print
+		std::cout << prefix << ": " << message << "\n";
+	}
+}
+
+void Log_Report( const std::string_view file, const std::string_view message, const std::string_view got, const std::string_view expected )
+{
+	if ( g_bUIReportMode )
+	{
+		const  auto line{ fmt::format( R"("report","{}","{}","{}","{}")", file, message, got, expected ) };
+		std::printf( "%s\n", line.c_str() );
+	}
+	else
+	{
+		const auto line{ fmt::format( "In file `{}`: {}", file, message ) };
+		std::fprintf( stderr, "%s\n", line.c_str() );
+	}
+}

src/log.hpp

@@ -0,0 +1,38 @@
+#pragma once
+
+#include <fmt/format.h>
+#include <fmt/chrono.h>
+
+enum class LogSeverity
+{
+	Info,
+	Warn,
+	Error,
+};
+
+// Base logging function
+void Log_Message( LogSeverity severity, const std::string_view message );
+
+// Report
+extern bool g_bUIReportMode;
+void Log_Report( const std::string_view file, const std::string_view message, const std::string_view got, const std::string_view expected );
+
+// Logging helpers
+template <typename... Ts>
+inline void Log_Info( const fmt::format_string<Ts...> fmt, Ts&&... args )
+{
+	Log_Message( LogSeverity::Info, fmt::format( fmt, std::forward<Ts>( args )... ) );
+}
+
+template <typename... Ts>
+inline void Log_Warn( const fmt::format_string<Ts...> fmt, Ts&&... args )
+{
+	Log_Message( LogSeverity::Warn, fmt::format( fmt, std::forward<Ts>( args )... ) );
+}
+
+template <typename... Ts>
+inline void Log_Error( const fmt::format_string<Ts...> fmt, Ts&&... args )
+{
+	Log_Message( LogSeverity::Error, fmt::format( fmt, std::forward<Ts>(args)... ) );
+}
+

src/main.cpp

@@ -6,15 +6,10 @@
 #include <memory>
 #include <vector>
 #include <argumentum/argparse.h>
-#include <fmt/format.h>
 
 #include "create.hpp"
-#include "output/CsvOutput.hpp"
-#include "output/RsvOutput.hpp"
-#include "output/JsonOutput.hpp"
-#include "output/Output.hpp"
-#include "output/SimpleOutput.hpp"
 #include "verify.hpp"
+#include "log.hpp"
 
 // the index file is encoded as `Rows-of-String-Values`
 // correct the index path with os
@@ -44,9 +39,7 @@ auto main( int argc, char* argv[] ) -> int {
 	std::string root{};
 	std::vector<std::string> excludes{};
 	std::string indexLocation{};
-	std::string format{};
 	bool overwrite{ false };
-	bool tee{ false };
 	const auto programFile{ std::filesystem::path( argv[ 0 ] ).filename() };
 
 	argumentum::argument_parser parser{};
@@ -74,67 +67,24 @@ auto main( int argc, char* argv[] ) -> int {
 		.metavar( "index-loc" )
 		.maxargs( 1 )
 		.absent( INDEX_PATH );
-	params.add_parameter( format, "--format", "-f" )
-		.help( "Output format. [simple, json, csv] default: `simple`" )
-		.metavar( "format" )
-		.choices( { "simple", "json", "csv", "rsv" } )
-		.maxargs( 1 )
-		.absent( "simple" );
 	params.add_parameter( overwrite, "--overwrite" )
 		.help( "Do not ask for confirmation for overwriting an existing index." )
 		.metavar( "overwrite" );
-	params.add_parameter( tee, "--tee" )
-		.help( "Also write the program's output to `$workDir/verifier.log`." )
-		.metavar( "tee" )
-		.absent( false );
+	params.add_parameter( g_bUIReportMode, "--ui-report" )
+		.help( "Use UI report mode logging." )
+		.metavar( "ui-report" );
 
 	if (! parser.parse_args( argc, argv, 1 ) ) {
 		return 1;
 	}
 
-	std::unique_ptr<Output> output;
-	switch ( format.at(0) ) {
-		case 's':
-			output = std::make_unique<SimpleOutput>();
-			break;
-		case 'j':
-			output = std::make_unique<JsonOutput>();
-			break;
-		case 'c':
-			output = std::make_unique<CsvOutput>();
-			break;
-		case 'r':
-			output = std::make_unique<RsvOutput>();
-			break;
-		default:
-			fmt::println( stderr, "Invalid `--format` argument: `{}`", format );
-			return 1;
-	}
 	int ret;
 
-	// `tee`, can be useful to track down a failure
-	FILE* file{ nullptr };
-	if ( tee ) {
-#ifndef _WIN32
-		file = std::fopen( "./verifier.log", "w" );
-#else
-		fopen_s( &file, "./verifier.log", "w" );
-#endif
-	}
-
-	output->init( file );
 	// `$basename started at $time` message
 	{
 		auto now{ std::time( nullptr ) };
-#ifndef _WIN32
-		auto local{ std::localtime( &now ) };
-		const auto line = fmt::format( "`{}` started at {:02d}:{:02d}:{:02d}", programFile.string(), local->tm_hour, local->tm_min, local->tm_sec );
-#else
-		tm local{};
-		localtime_s( &local, &now );
-		const auto line = fmt::format( "`{}` started at {:02d}:{:02d}:{:02d}", programFile.string(), local.tm_hour, local.tm_min, local.tm_sec );
-#endif
-		output->write( OutputKind::Info, line );
+		tm* local = std::localtime( &now );
+		Log_Info( "`{}` started at {:02d}:{:02d}:{:02d}", programFile.string(), local->tm_hour, local->tm_min, local->tm_sec );
 	}
 	if ( newIndex ) {
 		// stuff we ignore during the building of the index, the "standard" useless stuff is hardcoded
@@ -143,18 +93,15 @@ auto main( int argc, char* argv[] ) -> int {
 		excludes.emplace_back( ".*\\.vmx" );
 		excludes.emplace_back( ".*\\.log" );
 		excludes.emplace_back( ".*verifier_index\\.rsv" );
-		ret = create( root, indexLocation, excludes, overwrite, output.get() );
+		ret = create( root, indexLocation, excludes, overwrite );
 	} else {
-		// warn about stuff which shouldn't be here, don't use the `output::report` as this is a negligible user error
 		if ( overwrite )
-			fmt::println( stderr, "WARN: current action doesn't support `--overwrite`, please remove it." );
+			Log_Error( "current action doesn't support `--overwrite`, please remove it." );
 		if (! excludes.empty() )
-			fmt::println( stderr, "WARN: current action doesn't support `--exclude`, please remove it." );
+			Log_Error( "current action doesn't support `--exclude`, please remove it." );
 
-		ret = verify( root, indexLocation, output.get() );
+		ret = verify( root, indexLocation );
 	}
-	if ( file )
-		fclose( file );
 
 	return ret;
 }