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

ANSIStrings could handle spaces specially #21

Open
tbelaire opened this issue Aug 25, 2016 · 3 comments
Open

ANSIStrings could handle spaces specially #21

tbelaire opened this issue Aug 25, 2016 · 3 comments
Assignees

Comments

@tbelaire
Copy link

I see a potential improvement to ANSIStrings, where you can drop foreground colour changes for strings that are only whitespace.

I.E.

use ansi_term::Colour::Red;
use ansi_term::{ANSIString, ANSIStrings};
let some_value = format!("{:b}", 42);
let strings: &[ANSIString<'static>] = &[
    Red.paint("["),
    Blue.paint("   "),
    Red.paint("]"),
];
println!("Value: {}", ANSIStrings(strings));

could not bother switching to blue in the middle.

@ogham ogham self-assigned this Aug 27, 2016
@ogham
Copy link
Owner

ogham commented Aug 27, 2016

This is an interesting performance improvement! It's actually something that could be put to use in exa — there are quite a few places where it paints something in one colour, follows that with a default-coloured space, and then prints something in the same colour again. It could elide two of the colour codes if it knew that the spaces wouldn't actually affect anything.

the problem with doing it automatically is that it requires a runtime check, and that's not going to be appropriate in all cases. (Especially as we'd have to check for Unicode space characters, too.) Instead, I think this is possible with a two-step approach:

  1. Define a new style variant called Inherit or something that just keeps the colours that are already there. Currently it's not possible to do this because everything in ANSIStrings needs to have a style defined.
  2. Write a pass that detects strings that only contain spaces, and re-writes them to use this new style variant instead.

It would have to be clever and detect styles that do affect just spaces, such as underline or background colours, but it's certainly possible. How does this sound?

@ogham ogham closed this as completed Aug 27, 2016
@ogham ogham reopened this Aug 27, 2016
@ogham
Copy link
Owner

ogham commented Aug 27, 2016

whoops hit the wrong button

@joshtriplett
Copy link
Collaborator

I'd be hesitant to add this much cleverness around spaces, when the application code could also avoid styling the spaces. This would also take carefully analyzing things like "print styled space, position cursor, print over the space".

Does this case come up often enough to make it worth trying to omit the escape sequence?

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

3 participants