Skip to content

[Core] Refactor color_utilities.h to modern C++20#14312

Open
loumalouomega wants to merge 1 commit intomasterfrom
core/c++20-format-color-utilities
Open

[Core] Refactor color_utilities.h to modern C++20#14312
loumalouomega wants to merge 1 commit intomasterfrom
core/c++20-format-color-utilities

Conversation

@loumalouomega
Copy link
Copy Markdown
Member

📝 Description

Modernize color_utilities.h by replacing the preprocessor-only implementation with typed C++20 constructs (constexpr, std::string_view, std::format), while preserving full backward compatibility through legacy macro aliases.

What changed

Before

  • ~30 preprocessor #defines duplicated across #if !defined(_WIN32) / #else branches
  • Color codes defined as raw string literal macros (#define KRED "\x1B[31m")
  • Formatting macros relied on compile-time string literal concatenation (#define FRED(x) KRED x RST), which only works with string literals — not std::string or std::string_view
  • No type safety, no namespace scoping, no IDE discoverability

After

  • Kratos::ColorUtilities namespace with:
    • inline constexpr std::string_view constants for all ANSI codes (e.g. kRed, kBold, kReset)
    • [[nodiscard]] inline functions using std::format for text formatting (e.g. Red(), Bold(), Underline())
  • Platform-conditional #if/#else only applies once to the constants — no duplication of formatting logic
  • Backward-compatible macros (RST, KRED, FRED(x), BOLDFONT(x), etc.) are retained at the bottom and delegate to the namespace, so all existing call sites compile without changes

Why

Aspect Old New
Type safety None (macros) std::string_view params, std::string returns
Works with runtime strings No (literal concat only) Yes (std::string_view input)
Namespace pollution Global Scoped to Kratos::ColorUtilities
Code duplication Full duplication in #if/#else Constants only
IDE support Macros — no autocomplete/go-to-def Full IntelliSense support
Composability BOLDFONT(FGRN("text")) works by accident Bold(Green("text")) works by design

Impact

  • No breaking changes — all existing callers (KYEL, BOLDFONT(FGRN("CONVERGED")), GetStream() << RST, etc.) continue to compile
  • New code should prefer Kratos::ColorUtilities::Green("text") over FGRN("text")
  • Legacy macros can be removed in a future PR once callers are migrated

🆕 Changelog

@loumalouomega loumalouomega added Kratos Core C++ FastPR This Pr is simple and / or has been already tested and the revision should be fast Refactor When code is moved or rewrote keeping the same behavior labels Mar 20, 2026
@loumalouomega loumalouomega enabled auto-merge March 20, 2026 15:16
@loumalouomega loumalouomega requested a review from roigcarlo March 20, 2026 15:16
@loumalouomega
Copy link
Copy Markdown
Member Author

@roigcarlo as you said the C++20 support is not full, I will keep this until we update the Ubuntu images

@loumalouomega
Copy link
Copy Markdown
Member Author

@roigcarlo as you said the C++20 support is not full, I will keep this until we update the Ubuntu images

See #14291

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ FastPR This Pr is simple and / or has been already tested and the revision should be fast Kratos Core Refactor When code is moved or rewrote keeping the same behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant