From 5bd7cf18df939ca784c0bb54e77c0ab73bb80882 Mon Sep 17 00:00:00 2001 From: Thaddeus Crews Date: Sat, 21 Dec 2024 20:48:01 -0600 Subject: [PATCH] Core: Refactor `log_error` to unify color handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Can now detect & output color on CI --- core/io/logger.cpp | 44 +++++----- drivers/unix/os_unix.cpp | 48 ----------- drivers/unix/os_unix.h | 1 - platform/macos/macos_terminal_logger.mm | 16 +--- platform/windows/windows_terminal_logger.cpp | 84 -------------------- platform/windows/windows_terminal_logger.h | 1 - 6 files changed, 28 insertions(+), 166 deletions(-) diff --git a/core/io/logger.cpp b/core/io/logger.cpp index a0cf3e73ce13..fb61c0133894 100644 --- a/core/io/logger.cpp +++ b/core/io/logger.cpp @@ -32,6 +32,7 @@ #include "core/core_globals.h" #include "core/io/dir_access.h" +#include "core/os/os.h" #include "core/os/time.h" #include "modules/modules_enabled.gen.h" // For regex. @@ -55,34 +56,41 @@ void Logger::log_error(const char *p_function, const char *p_file, int p_line, c return; } - const char *err_type = "ERROR"; + const char *err_details = p_rationale && p_rationale[0] ? p_rationale : p_code; + + // Disable color codes if stderr is not a TTY or CI. + // This prevents Godot from writing ANSI escape codes when redirecting to a file. + const bool color = OS::get_singleton() && (OS::get_singleton()->has_environment("CI") || OS::get_singleton()->get_stderr_type() == OS::STD_HANDLE_CONSOLE); + const char *gray = color ? "\u001b[0;90m" : ""; + const char *red = color ? "\u001b[0;31m" : ""; + const char *red_bold = color ? "\u001b[1;31m" : ""; + const char *yellow = color ? "\u001b[0;33m" : ""; + const char *yellow_bold = color ? "\u001b[1;33m" : ""; + const char *magenta = color ? "\u001b[0;35m" : ""; + const char *magenta_bold = color ? "\u001b[1;35m" : ""; + const char *cyan = color ? "\u001b[0;36m" : ""; + const char *cyan_bold = color ? "\u001b[1;36m" : ""; + const char *reset = color ? "\u001b[0m" : ""; + switch (p_type) { - case ERR_ERROR: - err_type = "ERROR"; - break; case ERR_WARNING: - err_type = "WARNING"; + logf_error("%sWARNING:%s %s\n", yellow_bold, yellow, err_details); + logf_error("%s at: %s (%s:%i)%s\n", gray, p_function, p_file, p_line, reset); break; case ERR_SCRIPT: - err_type = "SCRIPT ERROR"; + logf_error("%sSCRIPT ERROR:%s %s\n", magenta_bold, magenta, err_details); + logf_error("%s at: %s (%s:%i)%s\n", gray, p_function, p_file, p_line, reset); break; case ERR_SHADER: - err_type = "SHADER ERROR"; + logf_error("%sSHADER ERROR:%s %s\n", cyan_bold, cyan, err_details); + logf_error("%s at: %s (%s:%i)%s\n", gray, p_function, p_file, p_line, reset); break; + case ERR_ERROR: default: - ERR_PRINT("Unknown error type"); + logf_error("%sERROR:%s %s\n", red_bold, red, err_details); + logf_error("%s at: %s (%s:%i)%s\n", gray, p_function, p_file, p_line, reset); break; } - - const char *err_details; - if (p_rationale && *p_rationale) { - err_details = p_rationale; - } else { - err_details = p_code; - } - - logf_error("%s: %s\n", err_type, err_details); - logf_error(" at: %s (%s:%i)\n", p_function, p_file, p_line); } void Logger::logf(const char *p_format, ...) { diff --git a/drivers/unix/os_unix.cpp b/drivers/unix/os_unix.cpp index 4a5c4dab6940..f123c2348e20 100644 --- a/drivers/unix/os_unix.cpp +++ b/drivers/unix/os_unix.cpp @@ -1045,54 +1045,6 @@ String OS_Unix::get_executable_path() const { #endif } -void UnixTerminalLogger::log_error(const char *p_function, const char *p_file, int p_line, const char *p_code, const char *p_rationale, bool p_editor_notify, ErrorType p_type) { - if (!should_log(true)) { - return; - } - - const char *err_details; - if (p_rationale && p_rationale[0]) { - err_details = p_rationale; - } else { - err_details = p_code; - } - - // Disable color codes if stdout is not a TTY. - // This prevents Godot from writing ANSI escape codes when redirecting - // stdout and stderr to a file. - const bool tty = isatty(fileno(stdout)); - const char *gray = tty ? "\E[0;90m" : ""; - const char *red = tty ? "\E[0;91m" : ""; - const char *red_bold = tty ? "\E[1;31m" : ""; - const char *yellow = tty ? "\E[0;93m" : ""; - const char *yellow_bold = tty ? "\E[1;33m" : ""; - const char *magenta = tty ? "\E[0;95m" : ""; - const char *magenta_bold = tty ? "\E[1;35m" : ""; - const char *cyan = tty ? "\E[0;96m" : ""; - const char *cyan_bold = tty ? "\E[1;36m" : ""; - const char *reset = tty ? "\E[0m" : ""; - - switch (p_type) { - case ERR_WARNING: - logf_error("%sWARNING:%s %s\n", yellow_bold, yellow, err_details); - logf_error("%s at: %s (%s:%i)%s\n", gray, p_function, p_file, p_line, reset); - break; - case ERR_SCRIPT: - logf_error("%sSCRIPT ERROR:%s %s\n", magenta_bold, magenta, err_details); - logf_error("%s at: %s (%s:%i)%s\n", gray, p_function, p_file, p_line, reset); - break; - case ERR_SHADER: - logf_error("%sSHADER ERROR:%s %s\n", cyan_bold, cyan, err_details); - logf_error("%s at: %s (%s:%i)%s\n", gray, p_function, p_file, p_line, reset); - break; - case ERR_ERROR: - default: - logf_error("%sERROR:%s %s\n", red_bold, red, err_details); - logf_error("%s at: %s (%s:%i)%s\n", gray, p_function, p_file, p_line, reset); - break; - } -} - UnixTerminalLogger::~UnixTerminalLogger() {} OS_Unix::OS_Unix() { diff --git a/drivers/unix/os_unix.h b/drivers/unix/os_unix.h index d331b0fb8e82..349fa2a8c2b0 100644 --- a/drivers/unix/os_unix.h +++ b/drivers/unix/os_unix.h @@ -111,7 +111,6 @@ class OS_Unix : public OS { class UnixTerminalLogger : public StdLogger { public: - virtual void log_error(const char *p_function, const char *p_file, int p_line, const char *p_code, const char *p_rationale, bool p_editor_notify = false, ErrorType p_type = ERR_ERROR) override; virtual ~UnixTerminalLogger(); }; diff --git a/platform/macos/macos_terminal_logger.mm b/platform/macos/macos_terminal_logger.mm index 44f37dc3961f..276d9efb4120 100644 --- a/platform/macos/macos_terminal_logger.mm +++ b/platform/macos/macos_terminal_logger.mm @@ -39,42 +39,30 @@ return; } - const char *err_details; - if (p_rationale && p_rationale[0]) { - err_details = p_rationale; - } else { - err_details = p_code; - } + StdLogger::log_error(p_function, p_file, p_line, p_code, p_rationale, p_type); + const char *err_details = p_rationale && p_rationale[0] ? p_rationale : p_code; switch (p_type) { case ERR_WARNING: os_log_info(OS_LOG_DEFAULT, "WARNING: %{public}s\nat: %{public}s (%{public}s:%i)", err_details, p_function, p_file, p_line); - logf_error("\E[1;33mWARNING:\E[0;93m %s\n", err_details); - logf_error("\E[0;90m at: %s (%s:%i)\E[0m\n", p_function, p_file, p_line); break; case ERR_SCRIPT: os_log_error(OS_LOG_DEFAULT, "SCRIPT ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", err_details, p_function, p_file, p_line); - logf_error("\E[1;35mSCRIPT ERROR:\E[0;95m %s\n", err_details); - logf_error("\E[0;90m at: %s (%s:%i)\E[0m\n", p_function, p_file, p_line); break; case ERR_SHADER: os_log_error(OS_LOG_DEFAULT, "SHADER ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", err_details, p_function, p_file, p_line); - logf_error("\E[1;36mSHADER ERROR:\E[0;96m %s\n", err_details); - logf_error("\E[0;90m at: %s (%s:%i)\E[0m\n", p_function, p_file, p_line); break; case ERR_ERROR: default: os_log_error(OS_LOG_DEFAULT, "ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", err_details, p_function, p_file, p_line); - logf_error("\E[1;31mERROR:\E[0;91m %s\n", err_details); - logf_error("\E[0;90m at: %s (%s:%i)\E[0m\n", p_function, p_file, p_line); break; } } diff --git a/platform/windows/windows_terminal_logger.cpp b/platform/windows/windows_terminal_logger.cpp index e25c612008c0..50de5fe8999e 100644 --- a/platform/windows/windows_terminal_logger.cpp +++ b/platform/windows/windows_terminal_logger.cpp @@ -30,8 +30,6 @@ #include "windows_terminal_logger.h" -#include "core/os/os.h" - #ifdef WINDOWS_ENABLED #include @@ -74,88 +72,6 @@ void WindowsTerminalLogger::logv(const char *p_format, va_list p_list, bool p_er #endif } -void WindowsTerminalLogger::log_error(const char *p_function, const char *p_file, int p_line, const char *p_code, const char *p_rationale, bool p_editor_notify, ErrorType p_type) { - if (!should_log(true)) { - return; - } - - HANDLE hCon = GetStdHandle(STD_OUTPUT_HANDLE); - if (OS::get_singleton()->get_stdout_type() != OS::STD_HANDLE_CONSOLE || !hCon || hCon == INVALID_HANDLE_VALUE) { - StdLogger::log_error(p_function, p_file, p_line, p_code, p_rationale, p_type); - } else { - CONSOLE_SCREEN_BUFFER_INFO sbi; //original - GetConsoleScreenBufferInfo(hCon, &sbi); - - WORD current_bg = sbi.wAttributes & (BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE | BACKGROUND_INTENSITY); - - uint32_t basecol = 0; - switch (p_type) { - case ERR_ERROR: - basecol = FOREGROUND_RED; - break; - case ERR_WARNING: - basecol = FOREGROUND_RED | FOREGROUND_GREEN; - break; - case ERR_SCRIPT: - basecol = FOREGROUND_RED | FOREGROUND_BLUE; - break; - case ERR_SHADER: - basecol = FOREGROUND_GREEN | FOREGROUND_BLUE; - break; - } - - basecol |= current_bg; - - SetConsoleTextAttribute(hCon, basecol | FOREGROUND_INTENSITY); - switch (p_type) { - case ERR_ERROR: - logf_error("ERROR:"); - break; - case ERR_WARNING: - logf_error("WARNING:"); - break; - case ERR_SCRIPT: - logf_error("SCRIPT ERROR:"); - break; - case ERR_SHADER: - logf_error("SHADER ERROR:"); - break; - } - - SetConsoleTextAttribute(hCon, basecol); - if (p_rationale && p_rationale[0]) { - logf_error(" %s\n", p_rationale); - } else { - logf_error(" %s\n", p_code); - } - - // `FOREGROUND_INTENSITY` alone results in gray text. - SetConsoleTextAttribute(hCon, FOREGROUND_INTENSITY); - switch (p_type) { - case ERR_ERROR: - logf_error(" at: "); - break; - case ERR_WARNING: - logf_error(" at: "); - break; - case ERR_SCRIPT: - logf_error(" at: "); - break; - case ERR_SHADER: - logf_error(" at: "); - break; - } - - if (p_rationale && p_rationale[0]) { - logf_error("(%s:%i)\n", p_file, p_line); - } else { - logf_error("%s (%s:%i)\n", p_function, p_file, p_line); - } - - SetConsoleTextAttribute(hCon, sbi.wAttributes); - } -} - WindowsTerminalLogger::~WindowsTerminalLogger() {} #endif // WINDOWS_ENABLED diff --git a/platform/windows/windows_terminal_logger.h b/platform/windows/windows_terminal_logger.h index 60d82bb93538..3e4ec8fc2c25 100644 --- a/platform/windows/windows_terminal_logger.h +++ b/platform/windows/windows_terminal_logger.h @@ -38,7 +38,6 @@ class WindowsTerminalLogger : public StdLogger { public: virtual void logv(const char *p_format, va_list p_list, bool p_err) override; - virtual void log_error(const char *p_function, const char *p_file, int p_line, const char *p_code, const char *p_rationale, bool p_editor_notify = false, ErrorType p_type = ERR_ERROR) override; virtual ~WindowsTerminalLogger(); };