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

Word diff via colored diffs #221

Open
mgeisler opened this issue May 30, 2023 · 5 comments
Open

Word diff via colored diffs #221

mgeisler opened this issue May 30, 2023 · 5 comments

Comments

@mgeisler
Copy link
Contributor

I would love to have colors in the diff output. Colors can be super helpful since they can make it possible to implement word-diffs in the terminal.

An example could look like this (this is from Magit in a text-mode Emacs):

image

@mgeisler
Copy link
Contributor Author

mgeisler commented Jul 6, 2023

Let me mention that the excellent pretty-assertions crate also supports this kind of highlighting:

image

@bjacotg bjacotg mentioned this issue Jul 17, 2023
@hovinen
Copy link
Collaborator

hovinen commented Jul 21, 2023

In order to have a robust solution for this, we'll need to increase the minimum supported Rust version from 1.59 to 1.64. This is because we need a robust way to determine whether it is safe to produce ANSI sequences for stdout, and the one crate I can find for this, supports-color, has transitive dependencies which impose this requirement. The alternative would be to make the dependency optional, but this would lead to obscure behaviour which I really want to avoid.

This will also require fixing an issue I have discovered in supports-color itself.

Are you absolutely certain that bumping the minimum supported Rust version is okay? I consider that a one-way street, so we should only include this feature if there is really no doubt.

@mgeisler
Copy link
Contributor Author

This is because we need a robust way to determine whether it is safe to produce ANSI sequences for stdout, and the one crate I can find for this, supports-color,

Can we not just use is_terminal as a start? That should do the usual thing which means colors when used in an interactive context and no colors otherwise.

@hovinen
Copy link
Collaborator

hovinen commented Jul 24, 2023

This is because we need a robust way to determine whether it is safe to produce ANSI sequences for stdout, and the one crate I can find for this, supports-color,

Can we not just use is_terminal as a start? That should do the usual thing which means colors when used in an interactive context and no colors otherwise.

That trait was made stable first in Rust 1.70.0, so we'd have to bump the MSRV even further.

We could directly use the crate which provides equivalent functionality, but its dependencies already require bumping the MSRV.

Either way, we'd then also lose the functionality provided by supports-color.

@mgeisler
Copy link
Contributor Author

Can we not just use is_terminal as a start? That should do the usual thing which means colors when used in an interactive context and no colors otherwise.

That trait was made stable first in Rust 1.70.0, so we'd have to bump the MSRV even further.

Ah, silly me... I knew I should have looked more carefully 😄

We could directly use the crate which provides equivalent functionality, but its dependencies already require bumping the MSRV.

Either way, we'd then also lose the functionality provided by supports-color.

Right, but you know how long it takes to import a new crate into AOSP. Personally, I would use atty instead which answers the "are you a TTY?" question on both Unix and Windows (it is part of AOSP). It might not support the environment variables, but it should do the right thing in the vast majority of cases.

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

No branches or pull requests

2 participants