Repository navigation
Fix how the log viewer reads ANSI escape codes - #7523
Open
itsnjstyle27 wants to merge 2 commits into
Open
itsnjstyle27 wants to merge 2 commits into
itsnjstyle27 wants to merge 2 commits into
Conversation
The log viewer only handled one SGR code per escape sequence and treated every escape sequence as one, so some common output showed up with the wrong style: - "ESC[m" (a reset without a code) didn't reset anything, so the color kept going until the end of the log. - Sequences with several codes like "ESC[0;32m" or "ESC[1;4;31m" were ignored. - Bright colors set with 90-97 and 100-107 were ignored. - Sequences that don't set a style, like "ESC[0K" (erase line) or "ESC[1A" (cursor up), were read as reset, bold, and so on. Now only sequences ending with "m" change the style, a missing code counts as a reset, and combined sequences apply each code in order. The arguments of 256 color and true color codes are skipped so they can't reset the style by accident. The existing "1;3x" bright colors work the same as before. Signed-off-by: img2024029-style <img_2024029@iiitm.ac.in>
itsnjstyle27
requested review from
Warashi,
ffjlabo,
hongky-1994 and
mohammedfirdouss
October 22, 2026 16:13
✅ Deploy Preview for pipecd-site canceled.
|
Contributor
|
👋 Hi @itsnjstyle27, welcome to PipeCD and thanks for opening your first pull request! We’re really happy to have you here Before your PR gets merged, please check a few important things below. Helpful resources
DCO Sign-offAll commits must include a In case you forget to sign-off your commit(s), follow these steps: For the last commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits: git rebase --signoff origin/master
git push --force-with-leaseRun checks locallyBefore pushing updates, please run: make checkThis runs the same checks as CI and helps catch issues early. 💬 Need help?If anything is unclear, feel free to ask in this PR or join us on the CNCF Slack in the #pipecd channel. Thanks for contributing to PipeCD! ❤️ |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved parsing and reverse-color issues can still reset unrelated styles, retain stale styles, or produce invalid palette indices.
3 open issues
What changed in this PR
This PR improves ANSI escape-code parsing in the deployment log viewer to address the color and style issues reported in #7517.
Changes:
- Handles empty resets and combined style codes while ignoring non-style sequences.
- Adds bright colors and skips unsupported extended-color arguments.
- Adds regression tests for the updated parsing behavior.
| File | Description |
|---|---|
web/src/utils/parse-log.ts |
Updates ANSI sequence handling and color support. |
web/src/utils/parse-log.test.ts |
Adds tests for resets, combined codes, bright colors, and ignored sequences. |
Review effort: Balanced
| // Codes that select a 256-color or a true color take extra arguments ("5;n" or | ||
| // "2;r;g;b"). Those colors aren't supported, but the arguments must be skipped so | ||
| // that a 0 in them isn't taken as a reset. | ||
| const EXTENDED_COLOR_CODES = ["38", "48"]; |
|
|
||
| // Set code for next cell process. | ||
| const code = match[1]; | ||
| const applyCode = (code: string): void => { |
| } | ||
|
|
||
| // Set code for next cell process. "ESC[m" is the same as "ESC[0m". | ||
| const code = match[1] ?? ANSI_CODE.RESET; |
…odes - Skip the arguments of SGR 58 (underline color) like 38 and 48, so a 0 in them is not taken as a reset. - Reverse now swaps the text and background colors instead of computing 7 - color, which gave negative indexes for bright colors. - Keep a leading empty code (ESC[;32m) in the match so it resets the style like ESC[0;32m. Signed-off-by: img2024029-style <img_2024029@iiitm.ac.in>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does:
Fixes how
parseLogreads ANSI escape codes in the deployment log viewer:mchange the style. Others, like erase line or cursor movement, are dropped without changing it.ESC[mcounts as a reset, same asESC[0m.ESC[0;32m,ESC[1;4;31m) apply each code in order.90-97and100-107are supported.38;5;n,38;2;r;g;b) are skipped, so a0in them isn't taken as a reset.The existing
1;3xbright colors work the same as before, and all existing snapshots are unchanged. Added tests for each of the cases above.Why we need it:
Without this, common output like
ESC[0;32mshows up with no color, a color started beforeESC[mruns until the end of the log, and progress output withESC[0Kloses its colors.Which issue(s) this PR fixes:
Fixes #7517
Does this PR introduce a user-facing change?:
Screenshots/Videos (for documentation or website changes):
N/A, no docs changes.