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

[Proposal] Stackable Formatting Tags #1623

Open
patrick96 opened this issue Jan 23, 2019 · 4 comments
Open

[Proposal] Stackable Formatting Tags #1623

patrick96 opened this issue Jan 23, 2019 · 4 comments
Labels
Projects
Milestone

Comments

@patrick96
Copy link
Member

patrick96 commented Jan 23, 2019

This is a proposal and specification for how we should implement the stackable formatting tags. I'll try to define the spec in a way that it doesn't break any existing configs that don't use raw tags (except for the output of the script module).

This got a little longer than I anticipated. When we implement this, I'll try to write a more concise spec for users and developers, but for actually implementing it, I think it should work well

The motivation behind this is to prevent issues like in #598, #937 or #1599 while not causing the bug described in #544 which occurred under the old stacking system (f754fa2).

The main issue is that %{F-} resets the color to the bar foreground color (same for all other tags that have a reset).
This means a label that has its foreground color set will reset the foreground for the entire module to the bar foreground.
If the foreground of the format is different from the bar foreground, subsequent drawtypes will have the wrong foreground color (unless they set one themselves).

The general idea is to make pushes and pops explicit, this way labels can push at the beginning and pop at the end and can thus restore the state as it was before the label started.

Syntax & Semantics

The main addition are two new special values for the formatting tags T, B, F, u, o, and U. The values now work as follows:

  • -: Reset the value to its default one (the one defined in the bar section)
  • *: Pops one value from the stack for that formatting tag, and set the current value for that formatting tag to the popped value
  • &: Pushes the value for the given formatting tag onto the stack for that formatting tag.
  • Everything else: As before,

This special value could be used like this:

%{F#f00} red %{F&}%{F#0f0} blue %{F#00f} green %{F-} root.foreground %{F*} red %{F-} root.foreground

Those special values also exist as attribute modifiers for u and o:

As before

  • +: Activate the attribute
  • -: Deactivate the attribute
  • !: Toggle the activation for the attribute

New

  • &: Push the activation for the attribute onto its stack
  • *: Pop the activation for the attribute from its stack and set it to the popped value

Example:

%{+u} underlined %{&u}%{-u} not underlined %{!u}%{!u} %{*u} underlined

So you see, everything done to the value between pairs of push and pop doesn't affect the value after the tag that pops from the stack. Popping completely resets the state for that tag.

Unfortunately we can't use more intuitive characters like + and - because they already have a meaning inside format tags. If you have ideas for more intuitive symbols, let me know.

You may have noticed that the action tag A doesn't get these new values, this is because action tags already have their own stacking system where %{A:...:} pushes and %{A} pops.

The reason I suggest pushing and popping values for tags individually instead of pushing the whole state at the beginning of a label and restoring it at the end is because currently, labels and other drawtypes only produces resetting tags (e.g. %{F-}) when there was a value set for it. This enables us to do things like the following in modules like the cpu module:

[module/cpu]
type = internal/cpu
format = <ramp-load><label>
label = %percentage%%
ramp-load-0 = %{F#00ff00}
ramp-load-1 = %{F#00ff00}
ramp-load-2 = %{F#00ff00}
ramp-load-3 = %{F#ffa500}
ramp-load-4 = %{F#ffa500}
ramp-load-5 = %{F#ffa500}
ramp-load-6 = %{F#ff0000}
ramp-load-7 = %{F#ff0000}

The foreground colors set via raw tags in the ramp bleed over to the label, changing it's color depending on the current cpu load. This kind of formatting is more expressive and we have also already recommended this on several occasions as a workaround for the missing label-warn in modules. To keep this behavior, we should (as before) only push and pop a certain tag, if there was a value set for it in the config.

Guarantees

I also want to define some guarantees we should provide users about when formatting tags are set and reset. Currently it's not really specified what guarantess are given, builder::flush could be called from arbitrary places and reset all open tags.
These guarantess can also be used as a spec to then write good integration tests.

I want to first define the notion of, what I will call, isolated units.

Isolated Unit:

An isolated unit is a component inside a module that gives the following guarantees:

  • font index, foreground color, background color, underline and overline color and their activations are the same directly before and directly after the isolated unit, unless they were modified by a user-supplied raw tag.
  • Isolated units will only set at the beginning and reset at the end formatting tags for properties that were explicitly set in the config defining this unit.

These guarantees will still allow bleeding over of raw tags, as I've described above but also don't have the issue that labels reset tags to the bar default, even though they shouldn't.

The following things should be isolated units:

  • Drawtypes. Since most drawtypes are just wrappers around displaying a single label, making labels an isolated unit, will probably automatically make all other drawtypes isolated units. For progressbars additionally bar-NAME-format needs to explicitly be made an isolated unit
  • The middle part of a format
  • Since format prefix and suffix are labels, the format as a whole is automatically an isolated unit.

Modules themselves should also be isolated units but with the stronger property that nothing inside the module can bleed out. This is already implemented in #1596

When implementing this we can remove the code that adds the tags for format-* both before and after the prefix (see #729). And we can only add the formatting tags before the prefix because the prefix should completely restore the state when it's finished.

Breaking Changes

The good news: If the user isn't using raw tags in the config, there should be no change in behavior (also not if they're only using script modules). The only change in behavior will be that faulty behavior is fixed. There may be instances where the faulty behavior is not apparent and the change to correct behavior is irritating to users.

If users actively used raw tags in labels and so on, I can't say for sure what behavior is going to change. The way I described the spec, there should be no case where a raw tag is reset earlier than it is now and only cases where the effects of the raw tags run on longer than before. I think having tags bleed over to the next label should still work properly. Also, tags that are set and reset in the same isolated unit should also not change their behavior.

I think breakage should be quite negligible, but we should properly document that we added certain guarantees and what those guarantees are. So I think this could be implemented in a minor as well.

Ref #598
Ref #937
Ref #1599
Ref #639
Ref #544
Ref #729
Ref #2814
Ref f754fa2

@Addisonbean
Copy link

Is this issue the same reason that using %{R#aaaaaa}...%{R-} resets colors that were set using format-...-foreground? Looking through some of the issues linked it seems to be the same underlying cause. I don't want to create a new issue if so

@patrick96
Copy link
Member Author

@Addisonbean %{R#aaaaaa} and %{R-} are not valid tags. The %{R} tag does not take a color, it just switches the current background and foreground color. But it shouldn't reset any colors. Are you really using the %{R} tag? The %{F-} tag would indeed reset the foreground color to the bar foreground and not the foreground defined in the format.

@Addisonbean
Copy link

Addisonbean commented Aug 21, 2020

You’re right! I had %{F#...} in my config but I typed it wrong over here. I should’ve copied it from the config

@patrick96
Copy link
Member Author

I came across this today again and I think we also need to address the issue where the user adds reset tags themselves.
For example:

label = Foo %{F#ff0000} Bar %{F-} Baz
label-foreground = #00ff00

The user wants to insert some text with a different foreground color in the middle of the label. But they have no way to reset the foreground color to the foreground color of the label without adding a separate tag that explicitly sets it to that color, they can only reset to the bar background.

Since we don't want to break backwards compatibility, we can't make the reset tag reset to the label color (or whatever color is at the top of the stack).
One possibility is to introduce an extra value that can peek (instead of pop), this would allow us to basically give the users a tag that can reset to the label color (because that will be on top of the stack).

The symbol for this could be =. So the tags would look like %{F=} and %{=u}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Refactoring
  
To Do
Development

No branches or pull requests

3 participants