Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no-color-fallback #31

Open
colin-kiegel opened this issue Feb 21, 2019 · 8 comments
Open

no-color-fallback #31

colin-kiegel opened this issue Feb 21, 2019 · 8 comments

Comments

@colin-kiegel
Copy link
Collaborator

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).
@Screwtapello
Copy link

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.

@Chewie
Copy link

Chewie commented Sep 8, 2021

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!

@tommilligan
Copy link
Collaborator

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.

@smwoj
Copy link

smwoj commented Feb 2, 2024

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?

@tommilligan
Copy link
Collaborator

Yes, I'd be very happy with that. There has been recent interest in this as well, ref #125

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

@smwoj
Copy link

smwoj commented Feb 6, 2024

@tommilligan could you please take a look at smwoj@73f051c 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

@tommilligan
Copy link
Collaborator

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.

@smwoj
Copy link

smwoj commented Feb 19, 2024

@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: #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants