-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Color customization #51
Comments
I think we should take the common subset that is supported by all platforms and expose them as English names. I think that means these: https://stebalien.github.io/doc/term/term/color/index.html --- I don't know precisely which ones are or aren't supported on Windows though. Things like "bold" or background colors don't work either (I think), or rather, bold "RED" is "BRIGHT_RED". So maybe we do something like: We should probably look at other tools that permit color customization and support both Linux and Windows and see what they do. I'd like to avoid specifying colors as ANSI escape codes. |
We could also do something a bit more generic that might be better for future expansion, like, If any option chosen is known not to be supported on Windows, then |
So it looks like Windows 10 supports ANSI codes, but I see no evidence of anything earlier than that supporting any non-full-window coloring at all. |
Yeah, it doesn't seem wise to jump on that so soon. We should stick with things that we know work. Maybe in a few years. :-) |
It might make more sense to do something like |
@cloudhead You'd actually need |
I quite like that syntax, I think that would be reasonable. And yeah, we can live with not customizing colors for Windows, but implementing for ANSI code-supporting terminals shouldn't be too bad. I'm thinking something like this:
Not sure if |
@eugene-bulkin I'm OK with I think we should absolutely support customizing colors for Windows. I just think we should have good failure modes if an end user tries to specify something that won't work (like, |
Yeah, agreed. Should be reasonable to display a warning, right? Just say "Your current terminal does not support [insert thing here]." |
I think it should be an error and cause |
Yeah, but is not being able to render a color they wanted really that worth killing the program? I view it as a soft error at best. But I don't have a problem with strict failure on all invalid input. |
I actually feel pretty strongly about this. If an end user asks ripgrep to do something, and ripgrep knows it can't, then it should just fail and tell the user how they can fix their problem. We should try to follow this philosophy everywhere in ripgrep. That's not to say that soft errors don't exist. One example of a soft error is if |
That's convincing enough for me. Makes sense! So:
I think that's a relatively sufficient overview of the changes needed. |
Since this kind of colorization is usually more of a somewhat permanent configuration thing rather than on a per-search basis, I think this should be settable via configuration file as well (although an alias will probably do just fine on Linux and Mac OS X for now, but not sure if that's a practical option on Windows). Whether or not ripgrep currently reads settings from any non-ignore files (I don't know actually), it seems at least to be a good idea to design a syntax that doesn't interfere with what parsers for, say, TOML or INI-style syntax can handle. Or maybe there's nothing to worry about? |
Let's please please avoid the question of a config file in this issue. There isn't actually an issue for a config file yet, but there probably should be. I'd like to at least try to resist the addition of a config file and see how far we can get with aliases.
Funnily enough, if you pass What I was think here was if the end user specified an attribute on windows. I think those just never work. But yes, otherwise that sounds OK. |
Since this behavior is already in |
I'd vote for making things compatible with Here's what I currently have: alias ag="ag --color-path '1;35' --color-line-number '0;37' --color-match '0;32' --color --break --group --heading" |
@zachriggle I'm strongly opposed to specifying colors with ANSI escape codes. That doesn't seem like good UX to me and will leave Windows users scratching their heads. |
I don't see why it can't be both. Specifying color /names/, without knowledge of the user's terminal theme, means the color mappings will be wrong. |
@zachriggle Could you please explain more? I'm not especially knowledgable about ANSI color escape codes. What would fail if we only asked for color names? |
Some people switch around colors in their terminal - for them "purple" does not necessarily mean purple. The color names will be correct for 99% (my guess) of people though. I think allowing both would be fine. May I also suggest allowing configuration of colors in a config file? My biggest annoyance with |
@anlutro I'd like to keep discussion of a config file out of this issue. A config file doesn't have its own issue yet, but it probably should. I'd like to resist the idea and see how far we can get with aliases, but I expect I won't be able to hold out forever. I kind of suspect that I'd like to punt on support ANSI escape codes for now. Not only because it seems like a pretty niche use case, but the term coloring library we're using presents a (mostly) platform independent abstraction, which isn't going to allow using ANSI escape codes directly. It's a much bigger change to support them. |
Just to add a voice to the conversation, here's what the colors look like under base16-twilight: It's nearly impossible to read the file name; the theme has switched the terminal's bold green for a color close to the background. Which is not your fault, I get, just a vote that I'd love to be able to customize the color. (Also: I really love ripgrep, thanks for the tool. I use search a lot more now that it's an order of magnitude faster. Hope this is helpful and not carping) |
@llimllib I don't think there's any question of whether we should allow color customization. I'm currently trying to tie off gitignore handling, which is taking longer than I hoped, but I hope to move into refactoring terminal related things like colors after that. |
OK, I'm finally ready to tackle this issue (and the litany of other color related issues). In particular, my plan is to jettison use of the I am going to move forward with this specification:
The
For example, this sets the match color to red and its style to bold, and set the line number background to green and not bold:
If you'd like to disable colors, then you can use the special
Color settings are applied iteratively. For example, to clear any existing color settings for
While the above specification doesn't resolve all use cases (e.g., "I want to set the exact ANSI escape code"), it does solve the most important ones (e.g., "I can't read the output of ripgrep") and dodges the question of platform differences by only supporting the intersection of ANSI color support and Windows console support. Most importantly, this specification leaves the door open for future expansion, such as additional text styles or supporting ANSI escape sequences directly. |
This commit completely guts all of the color handling code and replaces most of it with two new crates: wincolor and termcolor. wincolor provides a simple API to coloring using the Windows console and termcolor provides a platform independent coloring API tuned for multithreaded command line programs. This required a lot more flexibility than what the `term` crate provided, so it was dropped. We instead switch to writing ANSI escape sequences directly and ignore the TERMINFO database. In addition to fixing several bugs, this commit also permits end users to customize colors to a certain extent. For example, this command will set the match color to magenta and the line number background to yellow: rg --colors 'match:fg:magenta' --colors 'line:bg:yellow' foo For tty handling, we've adopted a hack from `git` to do tty detection in MSYS/mintty terminals. As a result, ripgrep should get both color detection and piping correct on Windows regardless of which terminal you use. Finally, switch to line buffering. Performance doesn't seem to be impacted and it's an otherwise more user friendly option. Fixes #37, Fixes #51, Fixes #94, Fixes #117, Fixes #182, Fixes #231
ANSI color codes don't support "just" bold, you have to specify which color to be bold. |
@CallumHoward I don't really know how to interpret your screenshot, but you can try, for example, |
@CallumHoward Ah I see now, thanks for the clarification! I think you're experiencing the same problem as in #266. |
The no-bold-escape-sequence output is not only a nuisance in terminal use, it is also a real problem for software trying to use |
@ilohmar Please file a new issue and describe the specific problem you're trying to solve. It would help if you left Emacs out of it completely (for example) because I don't use Emacs, and therefore I don't have the same context as you. |
Done, see #293. Emacs was just an example, sorry for not making this clear enough. |
Is there a way to do this in an environment variable or a settings file? |
For anyone coming here trying to make
|
We should definitely try to support customizing colors in the output similar to how
ag
does it. Currently they support 3 color customizations, as per this issue:--colors-match
--colors-path
--colors-line-number
Implementing the part where we set colors based on an argument is not particularly hard (just a matter of translating user input to
term::Attr
enums). Parsing input in the bash terminal format likeag
does was not difficult to implement. However, this limits Windows usage because we don't have an easy translation between those colors.How should we go about supporting these color customizations for both Linux, Mac and Windows?
The text was updated successfully, but these errors were encountered: