Skip to content

Conversation

@nevakrien
Copy link
Contributor

@nevakrien nevakrien commented Dec 10, 2025

as discussed in #160 adding a switch specifically for ansi colors is not ideal as it adds a dependency and slows down the usual path.
what this PR does is verify that its impossible to emit ansi colors under any config with color=false without manually inserting the ansi codes into the strings.

this is because:

  1. regardless of what ReportStyle gives as the color we filter it
  2. all XXX_color() methods filter
  3. all label colors are filtered at write time rather than when we add them

if I am reading correctly the old add_labels code this PR does introduce a breaking API change.
since previously setting the config after adding labels would make for those labels being colored.
I suspect this is not intuitive or desired behavior so I am considering this a bug fix.

I think this is much nicer since now users can be confident that setting color to off before printing would always generate output without ansi signs.

@zesterer
Copy link
Owner

This doesn't actually solve the same problem as the original solution. The reason that toggling colour is hard is because users will insert ANSI escape codes into things like error messages and label text, because that's what the crate encourages them to do. This change doesn't strip ANSI codes from the labels, and so most users will still see unintuitive results.

This is why I think that a proper solution requires work at the level of the formatting API as mentioned in #160.

@nevakrien
Copy link
Contributor Author

I think if we go with what we taljked about in #160 the issue of inline ANSI codes would still persists it would just be an anti pattern at that case. like other than manually parsing there is simply nothing we can do to avoid a user putting an ANSI charchter into the stream.

but other than that edge case I think the crate should not be adding them. so basically we would require users to not manually insert ansi colors ever. basically it seems there are 2 independent issues

  1. the crate sometimes adds colors to the given text even when color=false
  2. there are thigns that arent achivble without manually inserting ansi colors.

but if the hope is to solve both on the same PR i can do that as well. i had some ideas for styling

@zesterer
Copy link
Owner

zesterer commented Dec 10, 2025

I think if we go with what we taljked about in #160 the issue of inline ANSI codes would still persists it would just be an anti pattern at that case. like other than manually parsing there is simply nothing we can do to avoid a user putting an ANSI charchter into the stream.

Yes, but I don't think that's a problem. If the user is explicitly choosing to shoot themselves in the foot, there's not all that much that we can do about it, especially if we're providing steel-capped boots.

but if the hope is to solve both on the same PR i can do that as well. i had some ideas for styling

Yep, that's the goal. And I think it's possible!

Consider:

struct Styled<T> { inner: T, style: &'static str }

// Arbitrary, extremely unlikely to appear in any real strings
const STYLE_START: &'static str = "\x1B\x0E";
const ELEM_START: &'static str = "\x1B\x17";
const STYLE_END &'static str = "\x1B\x0F"; 

impl<T: fmt::Display> fmt::Display for Styled<T> {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "{STYLE_START}{}{ELEM_START}{}{STYLE_END}", self.style, self.inner)
    }
}

When the user does, say, format!("hello {}!", "world".style("error")) the resulting string is:

hello \x1B\x0Eerror\x1B\x17world\x1B\x0F!

When rendering a diagnostic, Ariadne will search for these escape sequences. When it finds one, it looks up the style name in the 'stylesheet' provided for the relevant backend (like the ANSI CLI one) and apply the relevant ANSI escape sequences to the text.

This has a few advantages:

  1. The escape sequence is easier to detect than ANSI ones, so we don't need to pull in heavyweight crates like vte to detect them

  2. This scales beyond the ANSI CLI output too. We can use styles for alternative backends that provide their own styling options. Here are some alternative backends I've been thinking about:

  • HTML+CSS (style gets translates to an inline DOM element with a CSS style tag)
  • LSP JSON (the style gets translated to a clickable link to, say, a location in the source code or some documentation)
  • Markdown (style gets translated into markdown **bold**/*italics*)

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.

2 participants