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

reveal.js: Explicitly setting boolean options to false fails #7402

Closed
salim-b opened this issue Jun 23, 2021 · 6 comments
Closed

reveal.js: Explicitly setting boolean options to false fails #7402

salim-b opened this issue Jun 23, 2021 · 6 comments
Labels

Comments

@salim-b
Copy link
Contributor

salim-b commented Jun 23, 2021

Problem

Since Pandoc 2.14.0.3 (more specifically 14b2eb2), all reveal.js config options are explicitly set to their default values.

While leaving the default of a boolean variable untouched (1) or flipping it to true (2) works fine, setting it to false (3) fails.

1

echo "---
title: 'Test'
---

# My presentation

blabla
" | pandoc --to=revealjs --from=markdown --self-contained | grep mouseWheel
        mouseWheel: false,

2

echo "---
title: 'Test'
mouseWheel: true
---

# My presentation

blabla
" | pandoc --to=revealjs --from=markdown --self-contained | grep mouseWheel
        mouseWheel: true,

3

echo "---
title: 'Test'
mouseWheel: false
---

# My presentation

blabla
" | pandoc --to=revealjs --from=markdown --self-contained | grep mouseWheel
        mouseWheel: ,

Workaround

Explicitly setting the boolean false as a string works:

echo "---
title: 'Test'
mouseWheel: 'false'
---

# My presentation

blabla
" | pandoc --to=revealjs --from=markdown --self-contained | grep mouseWheel
        mouseWheel: false,

Pandoc version

2.14.0.3

@salim-b salim-b added the bug label Jun 23, 2021
@jgm
Copy link
Owner

jgm commented Jun 23, 2021

The root problem is that the template engine (doctemplates) wasn't designed with boolean values in mind.
True gets rendered as "true", but False gets rendered as "". Why? Because the way conditionals work is that the condition is evaluated and tested for emptiness (empty = false, nonempty = true).

See jgm/doctemplates#16 -- ultimately we need to sort this out there.
In the mean time, it might be possible to change the writer so that it rewrites booleans as strings for these values (ugly).

@salim-b salim-b changed the title reveal.js: Explicitly setting boolean options to defaults fails reveal.js: Explicitly setting boolean options to false fails Jun 23, 2021
@jgm
Copy link
Owner

jgm commented Jun 23, 2021

One solution would be to

  • change the HTML writer so that the default values are real booleans (instead of strings 'true', 'false' as now)
  • change the templates like
mouseWheel: $if(mouseWheel)$true$else$false$endif$,

This is kind of ugly, but more straightforward than the present approach.

@jgm
Copy link
Owner

jgm commented Jun 23, 2021

Of course, that approach would break with the workaround above.
It would be nice if people could write either false or 'false' and have it work.

@jgm
Copy link
Owner

jgm commented Jun 23, 2021

Another possibility would be to add a "pipe" to doctemplates that converts a boolean value to the string true or false, leaves strings the same, and (does what?) with other types?
Or a more general pipe that turns a value into its canonical JSON representation?

@salim-b
Copy link
Contributor Author

salim-b commented Jun 23, 2021

One solution would be to

* change the HTML writer so that the default values are real booleans (instead of strings 'true', 'false' as now)

* change the templates like
mouseWheel: $if(mouseWheel)$true$else$false$endif$,

This is kind of ugly, but more straightforward than the present approach.

I guess this makes sense until underlying issue jgm/doctemplates#16 is resolved, thus see #7403.

jgm added a commit to jgm/doctemplates that referenced this issue Jun 23, 2021
Previously, `$if(foo)$` evaluated to false iff `foo`
would render as the empty string. This forced us to render
a boolean False value as an empty string, rather than `false`.
And this has caused various problems with templates
(#16, jgm/pandoc#7402).

This commit changes two things:

- Now, boolean False values render as `false` -- just as
  True values render as `true`.
- Conditionals are now sensitive to booleans, so
  `$if(foo)$` evaluates to false when `foo` is a boolean
  False value, even though it would render as the nonempty
  string `false`.

The README on conditionals has been updated accordingly.
@jgm
Copy link
Owner

jgm commented Jun 23, 2021

I've pushed commit 7dcf727
which uses a tweaked version of doctemplates. This seems to fix the problem.
(Of course, there's always the danger that it will cause other problems or break something for someone who was relying on false values rendering as empty strings...)

@jgm jgm closed this as completed Jul 18, 2021
salim-b added a commit to salim-b/revealjs-tpl that referenced this issue Jul 19, 2021
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Feb 12, 2022
0.10.0.1
* Don't rely on aeson Object being implemented as a HashMap. This change is
  needed for doctemplates to compile against aeson 2.0.0.0.

0.10
* Change rendering and conditional behavior with booleans. Previously,
  $if(foo)$ evaluated to false iff foo would render as the empty
  string. This forced us to render a boolean False value as an empty
  string, rather than false. And this has caused various problems with
  templates (#16, jgm/pandoc#7402). Now, boolean False values render as
  false -- just as True values render as true. And conditionals are now
  sensitive to booleans, so $if(foo)$ evaluates to false when foo is a
  boolean False value, even though it would render as the nonempty string
  false.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants