feat: Add the ability to configure per-module color styles #285

chipbuster · 2019-09-04T05:17:02Z

Description

Allows the user to specify styles on a per-module (and potentially even per-section) basis.

This has two main parts: the first is a new config function which can read a string specifier, e.g. underlined bold green bg:75 and return the appropriate ansi_term::Style object to match that specifier.

The second will be integrating this config function in with existing modules.

Motivation and Context

Closes #128 and #208

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Screenshots (if appropriate):

How Has This Been Tested?

I have tested using MacOS
I have tested using Linux
I have tested using Windows

Checklist:

I have updated the documentation accordingly.
I have updated the tests accordingly.

chipbuster · 2019-09-06T01:20:41Z

@starship/astronauts Looks like I have a working draft of this puppy (screenshot at end). The last thing I'd like to do before integrating it with our existing modules is to make sure the syntax is good.
@nickwb I'd also like your opinion, if you have one on the syntax here.

The current rules for the stylestring are as follows:

Valid tokens

The user can put bold, underline, fg, bg, or a color into the string, separated by spaces.
Nothing here is case sensitive.

Valid colors

A color can take on one of the three following forms:

A number between 0-255. This will be interpreted as an ansi color
A hash followed by 6 hexadecimal digits. This is interpreted as a hexadecimal RGB code.
A standard color string (e.g. red, blue, cyan, etc.)

Foreground/Background

Any color appearing after an fg will modify the text color, any color appearing after a bg modifies the background color.
If multiple colors appear to a specifier (e.g. fg red green bg blue purple), the last one on the line is applied (in this example, it's equivalent to fg green bg purple).
If neither fg nor bg appear in the string, all colors are assumed to apply to the foreground.

Bold and Italic

The tokens bold and italic may appear anywhere in the string, multiple times.

We can add additional styling codes that are supported by ANSI terminals (e.g. italic, dimmed, strikethrough), but given that a lot of terminal emulators don't support some of these codes, we might want to be judicious about what we support.

Does anyone have improvements to suggest for how this little DSL works before I bake it into the codebase? Do we want to add support for any additional styling?

nickwb · 2019-09-06T02:17:19Z

Looks like a good start @chipbuster, I like the different options you've provided for specifying colors.

Regarding the syntax, I do find it a little odd that bold are underline are independent from other tokens, but fg, bg and the colors are not.

For example, if you consider the current parser implementation, these strings are valid.

bg underline green
bg green underline red (Set's the bg twice!)

Some possible solutions:

Merge the tokens together: bg:green, fg:137
Have formal grammar rules which force a color to appear after fg or bg.

chipbuster · 2019-09-06T02:46:19Z

I like the colon solution. I originally had something similar in mind, but I guess I just derped because I didn't consider having separators (and things like fggreen look really weird).

Do you have any opinions on behavior if the same color is set twice? We can ignore subsequent settings, set to the last occurrence, or cause the parse to fail (presumably while logging a message).

matchai · 2019-09-06T03:46:44Z

A standard color string (e.g. red, blue, cyan, etc.)

How are bright vs normal colors handled? Would it be bright-red?

We can ignore subsequent settings, set to the last occurrence, or cause the parse to fail (presumably while logging a message).

This comes back to us not having an elegant solution for handling errors. Ideally, the error would only be logged once after a configuration change is made so that the prompt containing the error doesn't get in the way of the user trying to fix the issue. For the time being, we can simply respect the last occurrence, and eventually transition to error handling, like we have noted elsewhere in the codebase.

Does anyone have improvements to suggest for how this little DSL works before I bake it into the codebase?

Does anyone think that the DSL might be too complicated for advanced style configuration? I think it certainly would be simple for the most common use cases: green, bold red, etc.

Another option to allow for clearer control for advanced styling would be to use a nested table for style.

[[character]]
symbol = "👋"

  [character.style]
  foreground = "bold red"
  background = "blue"

Thoughts?

chipbuster · 2019-09-06T04:17:07Z

How are bright vs normal colors handled? Would it be bright-red?

Haven't thought that far ahead yet. There's actually not an explicit enum in ansi_term for bright colors, so I might just rework the match arms to return Fixed(0..15) instead of using the explicit color enums like I am right now.

For the time being, we can simply respect the last occurrence, and eventually transition to error handling

Sounds good to me.

Does anyone think that the DSL might be too complicated for advanced style configuration?

Do you mean too complicated in terms of for the user to understand, or for us to implement? I think with the change suggested by @nickwb the syntax becomes pretty simple to explain (whether it stays that way or not is another story)

chipbuster · 2019-09-07T01:47:06Z

Alright, the current language is the following:

underline, bold, bg:<color>, fg:<color>, or <color> (the last two are currently considered equivalent).
<color> can be:
- a name of one of the 8 basic colors, optionally prefixed with bright-
- a decimal number between 0-255 (ANSI colors)
- a # followed by a six-digit hex RGB code

We still take the last instance of any color if multiple colors are found in the format string.

Examples:

fg:bright-blue bg:bright-red sets bright blue text on a bright red background
bold underline fg:red sets bold underlined red text on the default background
fg:red #a3a3a3 sets a hexadecimal gray (the first color token is ignored)

docs/config/README.md

chipbuster · 2019-09-07T04:39:55Z

I accidentally introduced a bug which I think is useful as a feature: if the color config string is completely invalid, it acts the same way as a blank config (turns off all styling). I think this could be helpful if the user types in an invalid config string: they now know that starship has seen their update to the config, but that something is going wrong (as opposed to going to the default color, where you're not sure if its because starship didn't see the config change or not).

wyze

Looks great! Just had a question.

wyze · 2019-09-07T18:35:28Z

src/config.rs

+    let tokens = style_string.split_whitespace();
+    let mut style = ansi_term::Style::new();
+
+    // Should we color the foreground? If not, assume we color the background.


Is the sentence here correct? Seems like we default to foreground if no fg:/bg: token was found.

Ah, I think this is poorly worded.

The intent was to say that "if col_fg is false, then we're coloring the background", as opposed to "if col_fg is false, don't color anything".

The other option I had would have been to introduce an Enum for it, but that felt a little overkill for an either-or decision.

Thanks. It reads much better now.

matchai · 2019-09-07T19:18:01Z

Could we have an example where the style of a module is explicitly set to nothing?

[character]
style = "none"

chipbuster · 2019-09-07T19:21:45Z

@matchai Currently the way to do no styling is to set the style string to "". If you'd like I can add the none token to the list (I think I should add dimmed anyways, as it's used by the hostname module).

matchai

The PR looks great! Really looking forward to having this as part of the prompt! 😄

docs/advanced-config/README.md

matchai · 2019-09-07T19:23:47Z

docs/config/README.md

+- `"bg:blue fg:bright-green"` sets bright green text on a blue background
+- `"bold fg:27"` sets bold text with [ANSI color](https://i.stack.imgur.com/KTSQa.png) 27
+- `"underline bg:#bf5700"` sets underlined text on a burnt orange background
+- `""` explicitly disables all styling


Thoughts on using null or "none" here?

Suggested change

- `""` explicitly disables all styling

- `null` explicitly disables all styling

I've put in a new token none (as well as a new token dimmed).

matchai · 2019-09-07T19:26:46Z

docs/config/README.md

+- `""` explicitly disables all styling
+
+Note that what styling looks like will be controlled by your terminal emulator. For example, some terminal emulators will brighten the colors instead of bolding text, and some color themes use the same values for the normal and bright colors.
+
 ## Prompt


Could be addressed in a separate PR, but we should probably have style configuration under prompt.style for the prompt elements that aren't part of modules (e.g. prefix, suffix).

Let's do a separate PR. I'll open an issue once this gets merged.

src/config.rs

src/modules/username.rs

chipbuster mentioned this pull request Sep 6, 2019

Allow disabling ansi colors via config #208

Closed

chipbuster commented Sep 7, 2019

View reviewed changes

docs/config/README.md Outdated Show resolved Hide resolved

chipbuster marked this pull request as ready for review September 7, 2019 04:40

chipbuster requested a review from a team September 7, 2019 04:41

wyze approved these changes Sep 7, 2019

View reviewed changes

matchai approved these changes Sep 7, 2019

View reviewed changes

chipbuster and others added 14 commits September 7, 2019 19:08

Start writing color-parsing functions

2135eb1

Add color table, need to implement bg and 255

8628c07

Add color parsing function

eb19176

Finish draft of parsing functions

99012f4

Add tests for style parsing code

0c3ff3c

Working example of style config for username

46bea1a

Add fg/bg with colons

bf553a2

Add some comments

5da318d

Revert changing regular colors to Fixed

5ddd15f

Modify some modules to use new system

0e054f4

Update all modules to use styling from config

47df60c

Update docs

9bfdd16

Realphabetize config and add style docs

8c9eb90

Slight update to docs

b71de9a

chipbuster added 4 commits September 7, 2019 19:08

Add one test and minor doc tweak

560d410

Slight updates to comments and docs

e3567eb

Update comment about col_fg to be less confusing

618d2f7

Add support/tests/docs for none and dimmed tokens

0c37e66

chipbuster merged commit 9721666 into starship:master Sep 8, 2019

chipbuster deleted the color-config branch September 8, 2019 00:33

hoop33 mentioned this pull request Sep 12, 2019

Include italic styles #354

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add the ability to configure per-module color styles #285

feat: Add the ability to configure per-module color styles #285

chipbuster commented Sep 4, 2019 •

edited

Loading

chipbuster commented Sep 6, 2019 •

edited

Loading

nickwb commented Sep 6, 2019

chipbuster commented Sep 6, 2019 •

edited

Loading

matchai commented Sep 6, 2019

chipbuster commented Sep 6, 2019

chipbuster commented Sep 7, 2019 •

edited

Loading

chipbuster commented Sep 7, 2019

wyze left a comment

wyze Sep 7, 2019

chipbuster Sep 7, 2019

chipbuster Sep 7, 2019

wyze Sep 7, 2019

matchai commented Sep 7, 2019

chipbuster commented Sep 7, 2019

matchai left a comment

matchai Sep 7, 2019

chipbuster Sep 8, 2019

matchai Sep 7, 2019 •

edited

Loading

chipbuster Sep 7, 2019 •

edited

Loading

	- `""` explicitly disables all styling
	- `null` explicitly disables all styling

feat: Add the ability to configure per-module color styles #285

feat: Add the ability to configure per-module color styles #285

Conversation

chipbuster commented Sep 4, 2019 • edited Loading

Description

Motivation and Context

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Checklist:

chipbuster commented Sep 6, 2019 • edited Loading

Valid tokens

Valid colors

Foreground/Background

Bold and Italic

nickwb commented Sep 6, 2019

chipbuster commented Sep 6, 2019 • edited Loading

matchai commented Sep 6, 2019

chipbuster commented Sep 6, 2019

chipbuster commented Sep 7, 2019 • edited Loading

chipbuster commented Sep 7, 2019

wyze left a comment

Choose a reason for hiding this comment

wyze Sep 7, 2019

Choose a reason for hiding this comment

chipbuster Sep 7, 2019

Choose a reason for hiding this comment

chipbuster Sep 7, 2019

Choose a reason for hiding this comment

wyze Sep 7, 2019

Choose a reason for hiding this comment

matchai commented Sep 7, 2019

chipbuster commented Sep 7, 2019

matchai left a comment

Choose a reason for hiding this comment

matchai Sep 7, 2019

Choose a reason for hiding this comment

chipbuster Sep 8, 2019

Choose a reason for hiding this comment

matchai Sep 7, 2019 • edited Loading

Choose a reason for hiding this comment

chipbuster Sep 7, 2019 • edited Loading

Choose a reason for hiding this comment

chipbuster commented Sep 4, 2019 •

edited

Loading

chipbuster commented Sep 6, 2019 •

edited

Loading

chipbuster commented Sep 6, 2019 •

edited

Loading

chipbuster commented Sep 7, 2019 •

edited

Loading

matchai Sep 7, 2019 •

edited

Loading

chipbuster Sep 7, 2019 •

edited

Loading