Theme custom variables are not taken into account in all places of the theme file

[AlexanderZobkov]

I'm trying to use theme custom variablies feature to define re-usable values (brand colors, in my case).
By one of the colors I would like to define/change color of "link" elements.

If a color for "link" elements is defined via theme custom variables, defined color (let's say, red) is not taken into account and default color (black) is applied.

Probably other formatting elements are also affected by the issue.

It's easily reproducible on https://github.com/asciidoctor/asciidoctor-maven-examples/tree/master/asciidoctor-pdf-with-theme-example

Details:
custom-theme.yml
brand:
red: #FF0000
link:
font_color: $brand_red -> NOK, default color will be applied during PDF generation
...
link:
font_color: #FF0000 -> OK, specified color will be applied during PDF generation

[mojavelinux]

The problem/limitation here is with how we process hex color values. In YAML, the # indicates the start of a comment. We preprocess the YAML content looking for # in front of hex color values and remove it so that YAML won't ignore the value. The way we know we are looking at a color value is by the pattern _color or -color at the end of the key.

What's happening in your case is that YAML is seeing:

brand:
  red:

As a result, your link color ends up black.

If you want this to work, you have to either add _color to the end of the key name for any color in your brand:

brand:
  red_color: #FF0000

or you need to use the quoted form so that YAML won't drop the value:

brand:
  red: 'FF0000'

I suppose we could modify the preprocessor so that it also looks for key names that have color_ or color- so that you can write brand_color_red instead of brand_red_color. The other possible strategy is just to replace : #[A-Za-z0-9]{3,6}$ globally into the quoted form (e.g., 'FF0000').

[mojavelinux]

I added this information the theming guide. See https://github.com/asciidoctor/asciidoctor-pdf/blob/master/docs/theming-guide.adoc#custom-variables

[AlexanderZobkov]

Dan, thank you for describing how it work under the hood. There is a small issue with the newly added information, "#" symbold is not rendered correctly: " since -># is missing<- is the comment".

It looks like to support ability to define colors in hex form, semantics of certain elements of YAML format was changed/broken. I'm not sure that it's good approach as YAML format is not used in the form/way as it's supposed to. Example of potential consequences: can't edit and validate asciidoctor-pdf theme file in IDEs or YAML validation tools (like yamllint).

I would propose the following strategy:

1. Keep xxx_yyy_color: #FF0000 approach as it's now, for backward compatability. And avoid introduction of additional "color" suffixes or prefixes.
2. Discourage in the theming guide the use of the orginal approach in favor of xxx_yyy_color: '#FF0000' or xxx_yyy_color: 'FF0000' or xxx_yyy_color: FF0000 (probably too radical)
3. Add an example to the theming guide, in "Custom variables" section, that user can define colors in using the following forms: red: '#FF0000', green: 00FF00.

What do you think?

[mojavelinux]

@AlexanderZobkov I just sent a pull request to make the preprocessing of hex color values much more robust. See #382. With this change, the theme reader will be able to handle all values regardless of the key names and will honor inline comments that follow the value.

It looks like to support ability to define colors in hex form, semantics of certain elements of YAML format was changed/broken.

Correct. This was done to make the file feel more comfortable for designers (and people familiar with CSS, LESS, Sass, etc).

I'm not sure that it's good approach as YAML format is not used in the form/way as it's supposed to.

The theme file is not technically YAML format (since we also have inline variables). It is YAML-like, but it is also CSS-like (or, more accurately, LESS). We made the decision to take this approach to bridge the gap between the worlds of programming and design. Eventually, we may move to a custom parser and drop the use of YAML under the covers (so it's more CSS like, or at least, more like LESS or Sass).

Having said that...

can't edit and validate asciidoctor-pdf theme file in IDEs or YAML validation tools (like yamllint).

You have the choice of strictly adhering to YAML since, in the end, the file is being parsed by YAML.

Discourage in the theming guide the use of the orginal approach

The primary audience for the theming guide is a designer. For that reason, I like to encourage our custom style by default. But I have made it clear in the guide that there are three ways of doing it and why you would choose each.

in favor of xxx_yyy_color: '#FF0000' or xxx_yyy_color: 'FF0000'

These are the recommended styles if you want to be YAML friendly. Let's call them the developer styles.

or xxx_yyy_color: FF0000 (probably too radical)

In fact, YAML will mangle the value in many cases (because YAML reads it as a number if it contains only digits). That's one of the main reasons we introduced the leading #. See #158.

In the end, we need to allow all the forms, but I will make it clear in the guide that there are three options and why they exist.

[AlexanderZobkov]

Alright, intention is clear now.
Regarding red: '#FF0000' form, with publicly available asciidoctor libs, It gives green color :)

I hope it will be handled correctly with #382.

[mojavelinux]

Indeed, you will need to use master (just merged) or wait for next alpha.