fix blink

[Jason2866]

No description provided.

[coderabbitai]

Warning

Rate limit exceeded

@Jason2866 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 28 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c6dd1ba1-610f-4cc6-a8fa-63ae598e19f4

📥 Commits

Reviewing files that changed from the base of the PR and between f7ad6db and 190076a.

📒 Files selected for processing (1)

• src/util/console-color.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix_ansi_color

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Jason2866]

@coderabbitai full review and analyze if blink and ANSI color 256 is correct

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR details
gh pr view 104 --repo Jason2866/esp32tool

echo "=== PR DIFF ==="
gh pr diff 104 --repo Jason2866/esp32tool

echo "=== FILES CHANGED ==="
gh pr view 104 --repo Jason2866/esp32tool --json files -q '.files[].path'

Length of output: 925

---

🌐 Web query:

ANSI escape code blink SGR parameter correct value

💡 Result:

For ANSI/ECMA-48 SGR (Select Graphic Rendition), the blink parameters are:

• CSI 5 m — slow blink (less than 150 blinks/min) [1]
• CSI 6 m — rapid blink (150 blinks/min or more) [1]
• CSI 25 m — steady / blink off (cancels blink) [1]

Example: \x1b[5mblinking\x1b[25m (enable slow blink, then disable). [1]

Sources: [1] ECMA-48 SGR parameter list.

---

🏁 Script executed:

#!/bin/bash
# Read the full console-color.ts file
fd -e ts console-color --exec cat {}

Length of output: 18094

---

@Jason2866 Here is the full review and analysis of the blink and ANSI 256-color implementation:

---

✅ The Core Fix (for → while loop)

The PR converts the for loop into a while loop and moves ci++ to the end of the loop body. This is the right approach.

Why it matters: Inside case 38 and case 48, the code manually advances ci to skip over sub-parameters (ci += 2 for 256-color, ci += 4 for true-color). With the explicit ci++ at the bottom of the while body, it is immediately clear that the manual advances inside the cases are additional, not a replacement. The old for loop was semantically equivalent (no continue statements exist in the loop), but the intent was harder to reason about.

---

✅ Blink Implementation — Correct

Per ECMA-48 SGR:

SGR Code	Meaning	Implementation
5	Slow blink	blink = true, rapidBlink = false ✓
6	Rapid blink	rapidBlink = true, blink = false ✓
25	Blink off	both false ✓
0	Reset all	both false ✓

The CSS animations are also correct:

.log-blink       { animation: blink 0.4s step-end infinite; } /* ~150 blinks/min */
.log-rapid-blink { animation: blink 0.4s step-end infinite; }
`@keyframes` blink { 50% { opacity: 0; } }

The step-end timing function produces a hard on/off blink (not a fade), which is the correct terminal behaviour.

Root cause of the original blink bug: Before the case 38/case 48 lookahead logic was added, a sequence like \x1b[38;5;5m (256-color index 5 = magenta) would have caused ci to reach the 5 sub-parameter and trigger case 5 → spurious blink. The lookahead in case 38/48 now consumes all sub-parameters (via ci += 2 / ci += 4) before the ci++ at the bottom fires, preventing any sub-parameter from being re-processed as a top-level SGR code.

---

✅ ANSI 256-Color — Correct

Parameter consumption is correct:

Sequence	Parameters	ci advance in case	+ci++ at bottom	Total consumed
38;5;n	3	ci += 2	+1	3 ✓
38;2;r;g;b	5	ci += 4	+1	5 ✓
48;5;n	3	ci += 2	+1	3 ✓
48;2;r;g;b	5	ci += 4	+1	5 ✓

ANSI_256 color table is correct per the xterm-256color spec:

• Indices 0–15: Hardcoded standard/bright colors ✓
• Indices 16–231: 6×6×6 RGB cube using v ? v*40+55 : 0 formula ✓
• Indices 232–255: Grayscale ramp using i*10+8 ✓

---

⚠️ Minor Observations (not blockers)

1. log-fg-black renders as grey (rgb(128, 128, 128)), not true black. This is intentional for dark-terminal visibility but note that ANSI_256[0] = rgb(0,0,0) (true black), creating a slight inconsistency between SGR 30 and 38;5;0 paths:

   • SGR 30 → class log-fg-black → grey (rgb(128,128,128))
   • SGR 38;5;0 → ANSI_NAMED[0] → same class → same grey ✓ (consistent)

2. Incomplete true-color sequences (e.g. 38;2;r;g missing b): The code sets ci = codes.length - 1, then ci++ ends the loop. The partial sequence is silently ignored — acceptable behaviour.

3. 38;5;n with unknown sub-type (not 5 or 2): Falls through with no ci advance beyond the normal ci++, treating the sub-parameter as a new top-level SGR code. This is standard-compliant behaviour for unrecognised extended colour sequences.

---

Summary: The fix is correct. The while-loop refactor is safe and the blink (SGR 5/6/25) and 256-color (38;5;n, 48;5;n, true-color variants) implementations are all compliant with ANSI/ECMA-48.