Skip to content

Conversation

@strike-kokhotnikov
Copy link

@strike-kokhotnikov strike-kokhotnikov commented Nov 12, 2025

Description

Trivial fix of color log output.

What

Implementation discussion is here: #1136

Change type

What kind of change does this PR introduce?

  • [ X ] Bugfix
  • Feature
  • Code style update
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

Check list

  • [ X ] Code follows the style de facto guidelines of this project
  • Comments have been inserted in hard to understand places
  • Documentation has been updated (if necessary)
  • Test has been added (if possible)

@svc-nbu-swx-media
Copy link

Can one of the admins verify this patch?

@greptile-apps
Copy link

greptile-apps bot commented Nov 12, 2025

Greptile Overview

Greptile Summary

Fixed a logic bug where g_vlogger_log_in_colors was incorrectly initialized to true, causing ANSI color codes to be written to log files and pipes even when they shouldn't be.

Key Changes:

  • Changed initialization of g_vlogger_log_in_colors from MCE_DEFAULT_LOG_COLORS (true) to false
  • Removed unused includes (vma/util/utils.h and vma/util/sys_vars.h)

Impact:
Previously, when logging to a file or pipe (where isatty() returns false), the color flag would remain true from its initialization, resulting in ANSI escape sequences polluting non-terminal output. Now the flag correctly defaults to false and is only set to true when all conditions are met: valid file descriptor, TTY output, and user-requested colors.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix is straightforward and addresses a clear logic bug. Changing the initialization from true to false ensures color codes are only used when appropriate (TTY output). The removed includes were unused and safe to remove. No logical errors or side effects detected.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/vlogger/vlogger.cpp 5/5 Fixed initialization of g_vlogger_log_in_colors to prevent color codes in non-TTY output, removed unused includes

Sequence Diagram

sequenceDiagram
    participant Caller as VMA Main/Sock-Redirect
    participant vlog_start as vlog_start()
    participant Config as safe_mce_sys()
    participant System as isatty()/fileno()
    participant Global as g_vlogger_log_in_colors
    
    Note over Global: OLD: initialized to MCE_DEFAULT_LOG_COLORS (true)
    Note over Global: NEW: initialized to false
    
    Caller->>Config: get log_colors setting
    Config-->>Caller: returns user preference (true/false)
    Caller->>vlog_start: vlog_start(..., log_in_colors=user_pref)
    
    vlog_start->>System: fileno(g_vlogger_file)
    System-->>vlog_start: file_fd
    
    vlog_start->>System: isatty(file_fd)
    System-->>vlog_start: true (TTY) or false (file/pipe)
    
    alt file_fd >= 0 AND isatty(file_fd) AND log_in_colors
        vlog_start->>Global: g_vlogger_log_in_colors = log_in_colors
        Note over Global: Colors enabled (only for TTY)
    else condition not met
        Note over Global: OLD: remains true (BUGGY - colors in files!)
        Note over Global: NEW: remains false (CORRECT - no colors in files)
    end
    
    vlog_start-->>Caller: logger initialized
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants