Adding support for fill-opacity

[Mbodin]

I was in need of the fill-opacity attribute, so I did these changes. I haven't changed the parser yet, but I'm not sure where to start.

The attribute fill-opacity takes the same kind of argument than offset (a number or a percentage), but they mean different things: I think that it would not make sense to reuse the type offset (being either a number or a percentage), so I introduced a number_or_percentage type. Maybe we should instead declare several types for offset and fill_opacity?

[Drup]

we do tend to try to use semantics names for the various possible inputs, but that's to be evaluated ton a case by case basis. Remind me what are the semantics in the offset and the opacity cases ?

[Mbodin]

Right. Then it's better to define different types: I'll revert the renaming to offset and create a new type ☺

I think that offset relates to patterns: it moves the start of the pattern (for instance on dashed lines). If provided a percentage, then it's along the line for that percentage (and it can be negative or greater than 100%), if a number, it represents a distance (I don't know why one can't provide a unit there).

For fill-opacity, the percentage must be between 0% and 100% (it's just the opacity), and the number must be between 0 and 1. So it's not only used to represent different things (distance vs alpha), but also has different bounds.

I'm going to change this ☺

[Drup]

Given your description ... maybe we could just mandate a float between 0 and 1 for the opacity, actually. The two representations are redundant.

[Mbodin]

The attribute fill-opacity itself can be seen as redundant with style='fill-opacity:__;', but it is useful as least for parsing. Is it fine to parse both percentages and numbers, but only using number internally?

[Drup]

There are three things: 1) The API, 2) The representation 3) The parsing

Parsing should be complete (in this case, accepting both form), API should be convenient (In this case, avoid 15 layers of data format), and the internal representation should yield valid HTML5 (in this case, whatever works).

[Mbodin]

Great! So let's write it such that we are able to write both percentages and numbers, but only storing numbers internally, and only printing out numbers ☺

I changed things in reflect.ml to be able to use my parser. I'm really not used to this kind of things (it feels like writing Ltac definitions in Coq). I hope that I did not do anything too wrong.

By the way, I noticed that there is a fill_rule parser defined in syntax/attribute_value.ml which is unused in syntax/reflect/reflect.ml. Is it normal?

[Drup]

I changed things in reflect.ml to be able to use my parser. I'm really not used to this kind of things (it feels like writing Ltac definitions in Coq). I hope that I did not do anything too wrong.

Yeah, it's ... not very structured. It works surprisingly well in practice :D

By the way, I noticed that there is a fill_rule parser defined in syntax/attribute_value.ml which is unused in syntax/reflect/reflect.ml. Is it normal?

.... not sure. Probably not!

[Mbodin]

Sorry for the amount of commits: it took me some time to understand how everything works.

I'm starting to appreciate the point that I reached regarding the opacity attributes. What do you think?

[Drup]

Looks excellent 👍

I made some misc comments, and we can merge just after.

Note that the use of Re_str is not mandatory, just tends to yield quick implementation.