Skip to content

Conversation

@Lexedia
Copy link

@Lexedia Lexedia commented Dec 18, 2025

This PR adds RGB colour support for ANSI escape codes.
I wasn't sure how to approach this, so any suggestion is welcome.

Also for the changelog, I have no clue if it's the right way to do it considering 1.1.0 never got released.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Note: The Dart team is trialing Gemini Code Assist. Don't take its comments as final Dart team feedback. Use the suggestions if they're helpful; otherwise, wait for a human reviewer.

@Lexedia Lexedia requested a review from a team as a code owner December 18, 2025 20:15
@google-cla
Copy link

google-cla bot commented Dec 18, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request introduces RGB color support for ANSI escape codes, which is a valuable addition. The AnsiRgbCode class and rgb utility function are well-documented. The example and tests demonstrate the new functionality effectively. However, there are a few areas for improvement regarding robustness, correctness, and adherence to Dart's type system. Specifically, the _gradient function needs to handle single-character strings gracefully, the AnsiRgbCode class has a redundant and problematic field override, and the wrapWith function should prevent combining standard and RGB color codes of the same type. There are also minor issues with a typo in a test description and an error message.

Comment on lines +24 to +41
void _gradient(String text, bool forScript) {
final length = text.length;
final buffer = StringBuffer();
for (var i = 0; i < length; i++) {
final ratio = i / (length - 1);
int red, green, blue;
if (ratio < .5) {
red = ((1 - (ratio * 2)) * 255).round();
green = (ratio * 2 * 255).round();
blue = 0;
} else {
red = 0;
green = ((1 - ((ratio - .5) * 2)) * 255).round();
blue = (((ratio - .5) * 2) * 255).round();
}
buffer.write(rgb(red, green, blue).wrap(text[i], forScript: forScript));
}
print(buffer.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The _gradient function can produce NaN for ratio when text contains a single character (e.g., length is 1), leading to a runtime error due to division by zero. This can be fixed by handling the single-character case explicitly to prevent this issue, improving the robustness of the function.

Suggested change
void _gradient(String text, bool forScript) {
final length = text.length;
final buffer = StringBuffer();
for (var i = 0; i < length; i++) {
final ratio = i / (length - 1);
int red, green, blue;
if (ratio < .5) {
red = ((1 - (ratio * 2)) * 255).round();
green = (ratio * 2 * 255).round();
blue = 0;
} else {
red = 0;
green = ((1 - ((ratio - .5) * 2)) * 255).round();
blue = (((ratio - .5) * 2) * 255).round();
}
buffer.write(rgb(red, green, blue).wrap(text[i], forScript: forScript));
}
print(buffer.toString());
void _gradient(String text, bool forScript) {
final length = text.length;
if (length == 0) return;
final buffer = StringBuffer();
for (var i = 0; i < length; i++) {
final ratio = length == 1 ? 0.5 : i / (length - 1);
int red, green, blue;
if (ratio < .5) {
red = ((1 - (ratio * 2)) * 255).round();
green = (ratio * 2 * 255).round();
blue = 0;
} else {
red = 0;
green = ((1 - ((ratio - .5) * 2)) * 255).round();
blue = (((ratio - .5) * 2) * 255).round();
}
buffer.write(rgb(red, green, blue).wrap(text[i], forScript: forScript));
}
print(buffer.toString());
}

Comment on lines +192 to +195
if (code is AnsiRgbCode) {
ansiRgbCode = code;
} else if (code.code != -1) {
numericCodes.add(code.code);
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The wrapWith function currently allows both a standard foreground/background AnsiCode and an AnsiRgbCode of the same type to be present in the codes iterable. This can lead to ambiguous or incorrect ANSI escape sequences (e.g., ESC[34;38;2;255;0;0m). The foreground and background counters should also account for AnsiRgbCode instances to ensure only one color type is applied per foreground/background. This improves correctness and maintainability.

    if (code is AnsiRgbCode) {
      if (code.type == AnsiCodeType.foreground) {
        foreground++;
      } else if (code.type == AnsiCodeType.background) {
        background++;
      }
      ansiRgbCode = code;
    } else if (code.code != -1) {
      numericCodes.add(code.code);
    }

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.

1 participant