Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Header height fails with 3 digit number #555

Closed
meisterluk opened this issue Sep 12, 2016 · 1 comment
Closed

Header height fails with 3 digit number #555

meisterluk opened this issue Sep 12, 2016 · 1 comment
Assignees

Comments

@meisterluk
Copy link
Contributor

meisterluk@linux ~ % cat test.adoc
Hello World
===========

Test
meisterluk@linux ~ % echo "header:\n  height: 99\n" > default-theme.yml
meisterluk@linux ~ % asciidoctor-pdf -a pdf-stylesdir=. test.adoc  # fine
meisterluk@linux ~ % echo "header:\n  height: 100\n" > default-theme.yml
meisterluk@linux ~ % asciidoctor-pdf -a pdf-stylesdir=. test.adoc  # fail
undefined method `-' for "100":String
Did you mean?  -@
  Use --trace for backtrace
1 meisterluk@linux ~ % asciidoctor-pdf --trace -a pdf-stylesdir=. test.adoc
/var/lib/gems/2.3.0/gems/asciidoctor-pdf-1.5.0.alpha.12/lib/asciidoctor-pdf/converter.rb:1947:in `layout_running_content': undefined method `-' for "100":String (NoMethodError)
Did you mean?  -@
    from /var/lib/gems/2.3.0/gems/asciidoctor-pdf-1.5.0.alpha.12/lib/asciidoctor-pdf/converter.rb:175:in `convert_document'
    from /var/lib/gems/2.3.0/gems/asciidoctor-pdf-1.5.0.alpha.12/lib/asciidoctor-pdf/converter.rb:96:in `convert'
    from /var/lib/gems/2.3.0/gems/asciidoctor-1.5.4/lib/asciidoctor/document.rb:1044:in `convert'
    from /var/lib/gems/2.3.0/gems/asciidoctor-1.5.4/lib/asciidoctor.rb:1503:in `convert'
    from /var/lib/gems/2.3.0/gems/asciidoctor-1.5.4/lib/asciidoctor/cli/invoker.rb:94:in `block in invoke!'
    from /var/lib/gems/2.3.0/gems/asciidoctor-1.5.4/lib/asciidoctor/cli/invoker.rb:86:in `each'
    from /var/lib/gems/2.3.0/gems/asciidoctor-1.5.4/lib/asciidoctor/cli/invoker.rb:86:in `invoke!'
    from /var/lib/gems/2.3.0/gems/asciidoctor-pdf-1.5.0.alpha.12/bin/asciidoctor-pdf:31:in `<top (required)>'
    from /usr/local/bin/asciidoctor-pdf:23:in `load'
    from /usr/local/bin/asciidoctor-pdf:23:in `<main>'

Follow up from asciidoctor issue 1870

trim_height.class == "String"

@mojavelinux mojavelinux self-assigned this Sep 12, 2016
@mojavelinux mojavelinux added this to the v1.5.0.alpha.13 milestone Sep 12, 2016
@mojavelinux
Copy link
Member

I see what's happening. We're escaping possible color values before parsing the YAML and 100 is a valid hex color value, so it gets quoted. As we parse, we have no way to distinguish between a color value and a raw, 3-digit integer in the current implementation. (We could, of course, coerce it to a float when reading it, but that's leaking into the client).

The short-term solution is to recommend that numbers above two digits be written as a float (e.g., 100.0), a math expression (e.g, 1 * 100), or with a unit (e.g., 100pt). Otherwise, they get confused as color values. (If we're sticking with the CSS theme, it's not that much of a stretch to enforce the use of units.) I can make this recommendation in the theming guide.

I want to emphasize that the theme parser is not very robust. It's getting the job done for what we need right now. It will eventually need to be redone using a formal grammar.

fapdash pushed a commit to vogellacompany/asciidoctor-pdf that referenced this issue Dec 13, 2016
…re digits

- document that numbers with 3 or more digits my be confused as a color by preprocessor
- for example, 100 is interpreted as '100' (or #110000)
- solutions are proposed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants