Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strip ANSI escape codes from file logging #90900

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Apr 19, 2024

Text editors cannot display ANSI escape codes, so these should be stripped from log files to ensure readability.

Since this uses a regex, this works both for print_rich() and manually inserted ANSI escape codes.

This has a slight performance impact: starting Godot, printing 200,000 lines then exiting Godot now takes 1.85 seconds instead of 1.75 seconds on an i9-13900K with a Linux x86_64 optimized (LTO) editor build. It may be possible to have a slightly faster solution using manual string handling, but handling all ANSI escape codes in a robust manner can be nontrivial. I've tried to use strstr() to check for the presence of an ANSI escape code in the log message before applying the regex on it, but it made no difference in performance.

Preview

stdout appearance for reference:

image

Before

Log file contents:

raw string0
�[1mte�[91mst�[39m�[22m�[0m
1
�[1mte�[91mst�[39m�[22m�[0m
2
�[1mte�[91mst�[39m�[22m�[0m
3
�[1mte�[91mst�[39m�[22m�[0m
4

After

Log file contents:

test
1
test
2
test
3
test
4

@Calinou Calinou added this to the 4.x milestone Apr 19, 2024
@Calinou Calinou requested a review from a team as a code owner April 19, 2024 14:13
@Calinou Calinou added bug and removed enhancement labels Apr 19, 2024
@Calinou Calinou modified the milestones: 4.x, 4.3 Apr 19, 2024
@Calinou Calinou force-pushed the file-logging-strip-ansi-escape-codes branch from 8dad40e to 3175d62 Compare April 19, 2024 21:16
@Calinou Calinou force-pushed the file-logging-strip-ansi-escape-codes branch from 3175d62 to 12a004a Compare April 22, 2024 20:40
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good.

At some point we might have to consider making RegEx a core component instead of a module, as we're starting to use it in more and more places and these conditional behaviors are error prone. (It might be the case already that a build without the regex module, while it should compile fine, might have bugs in some features that are not expected.)

Text editors cannot display ANSI escape codes, so these should be
stripped from log files to ensure readability.

Since this uses a regex, this works both for `print_rich()`
and manually inserted ANSI escape codes.

Co-authored-by: bruvzg <7645683+bruvzg@users.noreply.github.com>
@Calinou Calinou force-pushed the file-logging-strip-ansi-escape-codes branch from 12a004a to 459f14c Compare April 23, 2024 23:02
@akien-mga akien-mga merged commit dcdaa7d into godotengine:master Apr 24, 2024
17 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

'print_rich' saves text to log with tags converted to ANSI symbols what makes closing tags unreadable.
4 participants