Skip to content

Show empty lines#103

Merged
Jason2866 merged 2 commits intomainfrom
timestamp_color
Apr 27, 2026
Merged

Show empty lines#103
Jason2866 merged 2 commits intomainfrom
timestamp_color

Conversation

@Jason2866
Copy link
Copy Markdown
Owner

@Jason2866 Jason2866 commented Apr 27, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved preservation of text formatting across multi-line console output
    • Enhanced timestamp application to consistently prefix each line in multi-line messages
    • Fixed handling of consecutive formatting codes in console display

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

Warning

Rate limit exceeded

@Jason2866 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 41 minutes and 8 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: 61e3e3d7-2723-429b-a7cd-4a86025f4cf9

📥 Commits

Reviewing files that changed from the base of the PR and between c803867 and 0bacab9.

📒 Files selected for processing (1)
  • src/util/timestamp-transformer.ts
📝 Walkthrough

Walkthrough

This PR modifies console color utilities and timestamp transformers in both JavaScript and TypeScript. The changes remove empty-string short-circuiting from span creation, enable leading ANSI SGR code detection and preservation, and shift timestamp prefixing from once-per-chunk to once-per-line for multi-line content.

Changes

Cohort / File(s) Summary
Console Color Utilities
js/util/console-color.js, src/util/console-color.ts
Removes empty-string short-circuit logic from addSpan, allowing span elements to be created and appended even for zero-length content segments between consecutive ANSI sequences.
Timestamp Transformers
js/util/timestamp-transformer.js, src/util/timestamp-transformer.ts
Detects and preserves leading ANSI SGR codes. Replaces single-chunk timestamp prepending with line-by-line prefixing for multi-line chunks, reconstructing output by splitting on newlines while re-applying the captured leading ANSI codes to each non-newline segment.

Sequence Diagram(s)

sequenceDiagram
    participant Input as Incoming Chunk
    participant OldFlow as Old Transformer
    participant NewFlow as New Transformer
    participant Output as Output

    Input->>OldFlow: chunk (possibly multi-line)
    OldFlow->>OldFlow: detect ANSI codes (ignored)
    OldFlow->>OldFlow: prepend timestamp once
    OldFlow->>Output: single timestamp + full chunk

    Input->>NewFlow: chunk (possibly multi-line)
    NewFlow->>NewFlow: detect & strip leading ANSI
    NewFlow->>NewFlow: split on newlines (preserve separators)
    loop For each non-newline line segment
        NewFlow->>NewFlow: prepend preserved ANSI + timestamp
    end
    NewFlow->>Output: multi-line output with per-line timestamps
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

Lines dance in colors bright and bold,
ANSI codes carefully retold,
Timestamps march on every line,
Spans emerge where none were seen before—
The rabbit's work shines evermore! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Show empty lines' is vague and does not clearly convey the main changes in the pull request, which involve modifications to ANSI color span handling and timestamp prefixing for multi-line content. Consider using a more descriptive title that reflects the primary changes, such as 'Handle empty ANSI spans and preserve leading codes in timestamp transforms' or 'Fix empty span rendering and multi-line timestamp prefixing'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch timestamp_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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/util/timestamp-transformer.ts (1)

57-78: Dead isFirstLine branch — both arms are identical.

In the loop, the if (isFirstLine) / else branches at lines 70‑76 produce exactly the same output (leadingAnsi + timestamp + " " + part), and isFirstLine is never read elsewhere. The flag and the conditional can be removed without behavior change, simplifying the loop considerably.

♻️ Proposed simplification
     // For multi-line chunks, we need to preserve ANSI codes on each line
     // Split on newlines, but keep the newline characters
     const lines = contentWithoutAnsi.split(/(\r?\n)/);
     let result = "";
-    let isFirstLine = true;
-
-    for (let i = 0; i < lines.length; i++) {
-      const part = lines[i];
+    for (const part of lines) {
       if (part === "\n" || part === "\r\n") {
-        // Newline separator - just append it
         result += part;
-        isFirstLine = false;
-      } else if (part === "") {
-        // Empty string from split, skip
-        continue;
-      } else {
-        // Actual content line
-        if (isFirstLine) {
-          // First line: ANSI codes + timestamp + content
-          result += leadingAnsi + timestamp + " " + part;
-        } else {
-          // Subsequent lines: ANSI codes + timestamp + content
-          result += leadingAnsi + timestamp + " " + part;
-        }
+      } else if (part !== "") {
+        result += leadingAnsi + timestamp + " " + part;
       }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/util/timestamp-transformer.ts` around lines 57 - 78, The isFirstLine flag
and its conditional are dead code: both branches append the same string; remove
the isFirstLine variable and its checks in the loop inside the timestamp
transformation logic and always handle content lines the same way (append
leadingAnsi + timestamp + " " + part to result). Keep the existing handling for
newline separators and empty strings (skip), and remove any assignment to
isFirstLine (and its declaration) to simplify the for-loop that iterates over
lines and constructs result using leadingAnsi, timestamp, and part.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/util/timestamp-transformer.ts`:
- Around line 57-78: The isFirstLine flag and its conditional are dead code:
both branches append the same string; remove the isFirstLine variable and its
checks in the loop inside the timestamp transformation logic and always handle
content lines the same way (append leadingAnsi + timestamp + " " + part to
result). Keep the existing handling for newline separators and empty strings
(skip), and remove any assignment to isFirstLine (and its declaration) to
simplify the for-loop that iterates over lines and constructs result using
leadingAnsi, timestamp, and part.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e0ae0945-58d0-46a3-9570-7a7f44612195

📥 Commits

Reviewing files that changed from the base of the PR and between 07d5f79 and c803867.

📒 Files selected for processing (4)
  • js/util/console-color.js
  • js/util/timestamp-transformer.js
  • src/util/console-color.ts
  • src/util/timestamp-transformer.ts
💤 Files with no reviewable changes (2)
  • src/util/console-color.ts
  • js/util/console-color.js

@Jason2866 Jason2866 merged commit 5fe59a8 into main Apr 27, 2026
3 checks passed
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.

1 participant