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

Pandoc 2.x compatability #149

Closed
davidski opened this issue Nov 5, 2017 · 17 comments
Closed

Pandoc 2.x compatability #149

davidski opened this issue Nov 5, 2017 · 17 comments

Comments

@davidski
Copy link

@davidski davidski commented Nov 5, 2017

Pandoc 2.x seems to break flexdashboard's layout engine. orientation: rows does not take effect and column/row headers appear to not be parsed correctly. Dropping back to pandoc 1.19 fixes this problem. Running the github version of rmarkdown (required for pandoc 2.x) has no effect on this issue.

@jjallaire
Copy link
Member

@jjallaire jjallaire commented Nov 5, 2017

@bborgesr and @yihui Could you take a look at this?

@bborgesr
Copy link
Contributor

@bborgesr bborgesr commented Nov 5, 2017

Here's the unfortunate repro:

---
title: "Row Orientation"
output: 
  flexdashboard::flex_dashboard:
    orientation: rows
---

```{r, include = FALSE}
library(ggplot2)
library(dplyr)
```

```{r}
paste("Pandoc version:", rmarkdown::pandoc_version())
```

Row
-------------------------------------
    
### Chart 1
    
```{r}
DT::datatable(mtcars)
```
   
Row
-------------------------------------
    
### Chart 2
    
```{r}
ggplot(mtcars, aes(mpg, wt)) + geom_boxplot()
```
    
### Chart 3

```{r}
ggplot(mtcars, aes(mpg, wt)) + geom_point()
```

Pandoc 1.19.2.1:
screen shot 2017-11-05 at 4 31 33 pm

Pandoc 2.0.1.1:
screen shot 2017-11-05 at 4 35 53 pm

@yihui
Copy link
Member

@yihui yihui commented Nov 6, 2017

It might be a similar issue as rstudio/bookdown#479. I'm not really familiar with the codebase of flexdashboard, so I'm not sure.

@davidski We should definitely make this package compatible with Pandoc 2.0 at some point. At the moment, do you absolutely need Pandoc 2.0?

@jjallaire
Copy link
Member

@jjallaire jjallaire commented Nov 6, 2017

@yihui In some cases I think pandoc 2.0 is coming along for the ride as a result of brew updates (since rmarkdown will always use the latest version of pandoc found). We could proactively defend against this by masking out pandoc 2.0 here: https://github.com/rstudio/rmarkdown/blob/master/R/pandoc.R#L513

@davidski
Copy link
Author

@davidski davidski commented Nov 6, 2017

JJ is correct. I picked up the pandoc 2.0 train via homebrew on my Mac. 2.x is about to hit Windows as well via chocolatey so a number of both Mac & Windows package management users will facing these compatibility gotchas soon

@jjallaire
Copy link
Member

@jjallaire jjallaire commented Nov 6, 2017

I wonder if it's occurred to any of the package manager maintainers that pandoc v2 should be it's own package (e.g. pandoc2) as sliding a backwards incompatible release in under users is going to break tons of already working documents.

@davidski
Copy link
Author

@davidski davidski commented Nov 6, 2017

That’s a much more reasoned version of what I was thinking while trying to roll back. 😛

@jcheng5
Copy link
Member

@jcheng5 jcheng5 commented Nov 6, 2017

I think masking out pandoc 2.0 in find_pandoc is a great idea, at least as a stopgap measure. @yihui would you have time to take a stab at that? (I'm on vacation this week or I would offer to do it myself)

@bborgesr in the meantime can you try to isolate the relevant change in the generated HTML?

@yihui
Copy link
Member

@yihui yihui commented Nov 6, 2017

I think it will more flexible if we provide a way for users to specify which version of Pandoc they want to use (e.g. options(rmarkdown.pandoc.version = "1.19.2.1")), instead of forcibly masking out a certain version of Pandoc, because there might be users who actually want Pandoc 2.0.

This Pandoc 2.0 issue is currently of highest priority on my plan for this week, so yes, I'll definitely take a stab at it, and I'm aiming at an rmarkdown release by the end of this week.

@jcheng5
Copy link
Member

@jcheng5 jcheng5 commented Nov 6, 2017

@yihui Oh, OK. Sounds good.

@jjallaire
Copy link
Member

@jjallaire jjallaire commented Nov 6, 2017

@yihui Then in that case we should also make sure to get bookdown, flexdashboard, etc. working with pandoc 2.0 in the same timeframe (I don't think it's fair to users if we allow their documents to be broken from under them and then require they discover the fix on GitHub / SO /etc.).

Net: I think we either need to be globally conservative (don't allow pandoc 2.0 by default) or globally permissive (get all of our packages / formats working with pandoc 2.0). I am fine with either, I just don't want the in between of allowing the breakage and then saying we will fix the broken formats "at some point" or "soon".

@yihui
Copy link
Member

@yihui yihui commented Nov 7, 2017

@jjallaire That makes sense. I tend to be globally permissive since we have to do it sooner or later. I need to evaluate how difficult it is to fix this issue and the bookdown issue. The rmarkdown package should be good to go with Pandoc 2.0. I think the number of flexdashboard and bookdown users should be a relatively small fraction of the number of rmarkdown users, so it is probably not too bad. From what I have learned, those who are using Pandoc 2.0 didn't really mean to use it -- it is just a consequence of an unconscious brew upgrade, and they didn't know RStudio bundled Pandoc in the IDE.

@yihui yihui mentioned this issue Nov 7, 2017
3 of 3 tasks complete
@jjallaire
Copy link
Member

@jjallaire jjallaire commented Nov 7, 2017

No matter how many flexdashboard or bookdown users there are, I don't think we should allow them to be broken unwittingly when we can do something to prevent it. So I think we should either: (a) Hold the rmarkdown release until we fix the flexdashboard and bookdown issues; or (b) Actively prevent the use of pandoc v2.0 within rmarkdown for this release, then release again in one month with all packages fixed and pandoc v2.0 enabled.

@yihui
Copy link
Member

@yihui yihui commented Nov 8, 2017

The rmarkdown package should be fully compatible with Pandoc 2.0 now. I just fixed the bookdown issue, too. The only known issue left is this one in flexdashboard. I'll investigate it in the afternoon. The worst case is to prevent Pandoc 2.0 in flexdashboard if I cannot figure out a fix.

@jjallaire
Copy link
Member

@jjallaire jjallaire commented Nov 8, 2017

Okay, that sounds good to me!

@yihui
Copy link
Member

@yihui yihui commented Nov 9, 2017

Good news is that the fix turned out to be simple enough: #150.

@DataStrategist
Copy link

@DataStrategist DataStrategist commented Jun 7, 2018

Just to be super explicit, in case anyone has this bug, I resolved by reinstalling both rmarkdown and flexdashboard to the CRAN versions as of today. Thanks yihui 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.