rust-pretty-assertions icon indicating copy to clipboard operation
rust-pretty-assertions copied to clipboard

no-color-fallback

Open colin-kiegel opened this issue 6 years ago • 8 comments

ANSI colours aren't available ... a) when the compiler output is piped, e.g. into an editor or IDE b) in some terminals without the support

Much like the Rust compiler's warnings are still useful when colour is disabled, it would be nice if this crate's output was just as useful without colour output.

Option 1

  Some(
      Foo {
-         lorem: "Hello World!",
+         lorem: "Hello Wrold!",
          ipsum: 42,
          dolor: Ok(
-             "hey"
+             "hey ho!"
          )
      }
  )
  • PRO: unambiguous
  • CON: minor differences hard to spot (especially in long lines)

Option 2

Do it like Pythons unit test framework

  Some(
      Foo {
-         lorem: "Hello World!",
?                         -
+         lorem: "Hello Wrold!",
?                        +
          ipsum: 42,
          dolor: Ok(
-             "hey"
+             "hey ho!"
?                 ++++
          )
      }
  )
  • PRO: unambiguous
  • PRO: extra hint to spot minor differences
  • CON: soft line-wraps will turn this into a mess for long lines

Option 3

use some special chars

  Some(
      Foo {
-         lorem: "Hello W[-r-]old!",
+         lorem: "Hello Wo{+r+}ld!",
          ipsum: 42,
          dolor: Ok(
-             "hey"
+             "hey{+ ho!+}"
          )
      }
  )
  • PRO: extra hint to spot minor differences - even for long lines
  • CON: ambiguous, because you could think [--] and {++} are part of the strings
  • CON: hard to read, if lines contain a lot of special characters

Open Questions

  • can we detect ANSI-colour support automatically? (note: if yes, we have to be carfeul about unit tests in situations without ANSI-colour support. This should be an exception to still make the tests pass).

colin-kiegel avatar Feb 21 '19 21:02 colin-kiegel

can we detect ANSI-colour support automatically?

In general, no. stdout is just a byte-sink, and there's no way to know what the process at the other end will do with it. However:

  • the atty crate can tell you whether a given stream is connected to a terminal or not. If it's not connected to a terminal then probably you shouldn't send ANSI codes.
  • if it is connected to a terminal, it probably supports ANSI colour, but it might support a different set of codes, or nothing at all. Generally the $TERM environment variable says what kind of terminal it is, and the system's terminfo database (available via the term crate) tells you whether it can display colours and how.
  • However, although terminfo is The Right Thing, it can cause problems with tools like less that do not do the right thing.

Screwtapello avatar Feb 21 '19 23:02 Screwtapello

Hello, I wanted to know if this feature was still worked on? I'm still a beginner in Rust, but I'd gladly help if I can!

Chewie avatar Sep 08 '21 19:09 Chewie

Hey, I'm open to helping out anyone wanting to take on the work and chat about an overall design - I'm unlikely to work on it myself in the near future.

As a vague outline, I would be happy with a solution that:

  • uses atty as a heuristic to decide whether to use color (we just need a rough guide, and term is massive)
  • looks like Option 1/Option 2 as outlined above (I think the implementation of these styles will be quite similar, and happy to bikeshed nearer the time)
  • only adds overhead in the cases where a diff needs to be printed, not when the assertion passes

@Chewie if you'd like to take a look at it I'm happy to help out or explain anything I can - feel free to ping me a draft PR even if you still have questions or it's unfinished.

tommilligan avatar Sep 09 '21 17:09 tommilligan

I'd be happy to revive this issue and help with this, if you are still open to that, @tommilligan.

uses atty as a heuristic to decide whether to use color (we just need a rough guide, and term is massive)

Standard library now has equivalent functionality in the form of IsTerminal trait, so perhaps we could use that instead?

smwoj avatar Feb 02 '24 14:02 smwoj

Yes, I'd be very happy with that. There has been recent interest in this as well, ref https://github.com/rust-pretty-assertions/rust-pretty-assertions/issues/125

I have more bandwidth to help with reviews/design than writing code at the moment :+1:

tommilligan avatar Feb 02 '24 14:02 tommilligan

@tommilligan could you please take a look at https://github.com/smwoj/rust-pretty-assertions/commit/73f051c393e31eca2c2f48500b52d0406777e8de and let me know what you think?

I tried to keep the diff focused, because I think adding more configurability or abstractions would conflict with existing initiatives in this repo.

Examples of cargo --quiet run --example pretty_assertion 2> out

using linked commit
assertion failed: `(left == right)`

Diff < left / right > :
 Some(
     Foo {
<        lorem: "Hello World!",
>        lorem: "Hello Wrold!",
         ipsum: 42,
         dolor: Ok(
<            "hey",
>            "hey ho!",
         ),
     },
 )

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
using master branch
thread 'main' panicked at pretty_assertions/examples/standard_assertion.rs:20:5:
assertion failed: `(left == right)`

[1mDiff[0m [31m< left[0m / [32mright >[0m :
 Some(
     Foo {
[31m<        lorem: "Hello W[0m[1;48;5;52;31mo[0m[31mrld!",[0m
[32m>        lorem: "Hello Wr[0m[1;48;5;22;32mo[0m[32mld!",[0m
         ipsum: 42,
         dolor: Ok(
[31m<            "hey",[0m
[32m>            "hey[0m[1;48;5;22;32m ho![0m[32m",[0m
         ),
     },
 )

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

smwoj avatar Feb 06 '24 10:02 smwoj

It looks great to me. I wonder if there's an even more generic abstraction we can make (such that everything to be styled goes through a single call), but I think what you've suggested looks good.

I would like to see how this interacts with tests/CI, but very on board with what you've linked.

tommilligan avatar Feb 06 '24 23:02 tommilligan

@tommilligan apologies for this delay, but I didn't have capacity to drive this change in the past 2 weeks. I'll have some capacity this week, so I opened a PR: https://github.com/rust-pretty-assertions/rust-pretty-assertions/pull/127.

In addition, I checked what a very similar crate does in this case, because it does handle this case gracefully. It styles the output using console crate, which also internally uses libc::isatty (same as IsTerminal trait from the standard library) to determine whether output should be colored, but can also be configured with environment variables.

Switching to console for pretty-assertions styling needs would be another approach to solve this problem. If you prefer that approach (it would also have other benefits, see the configuration with env vars) I could prepare such patch, too. console has very minimalistic dependency tree.

smwoj avatar Feb 19 '24 14:02 smwoj