quarto-dev / quarto

Quarto open-source scientific and technical publishing system
https://quarto.org
GNU Affero General Public License v3.0
305 stars 25 forks source link

Rmd handling in Positron is broken when there is a triple backtick immediately followed by prose on the next line #493

Open DavisVaughan opened 1 month ago

DavisVaughan commented 1 month ago

See https://github.com/posit-dev/positron/issues/4155

Create an Rmd with the following in it


Lets say you have two variables `x` and `y` that both point to the same underlying data.

```{r}
x <- c(1, 2, 3)
y <- x

If you modify y, R will first copy the values of x to a new position, then point y to the new location and only after the copy modify y.

x <- 1

Note the triple backticks _immediately_ followed by `If you` on the next line (no newline between them). This seems to cause issues.

Place your cursor on `x <- 1` and try to run that line with `Cmd + Enter`, it will instead send this massive block to the console (in Positron):

If you modifyy, R will first copy the values ofxto a new position, then pointyto the new location and only after the copy modifyy`.

x <- 1

Also notice how the highlighting is off here:

<img width="895" alt="Screenshot 2024-07-25 at 3 46 36 PM" src="https://github.com/user-attachments/assets/80f77303-03d4-430f-8929-b0cffcccdcc4">

A triple backtick immediately followed by prose on the next line works fine in RStudio (and highlights fine) so I think it should be supported here too.

```bash
quarto check

[✓] Checking versions of quarto binary dependencies...
      Pandoc version 3.1.2: OK
      Dart Sass version 1.55.0: OK
      Deno version 1.33.2: OK
[✓] Checking versions of quarto dependencies......OK
[✓] Checking Quarto installation......OK
      Version: 1.4.185
      Path: /Applications/quarto/bin

[✓] Checking basic markdown render....OK

[✓] Checking Python 3 installation....OK
      Version: 3.12.2
      Path: /Users/davis/.pyenv/versions/3.12.2/bin/python3
      Jupyter: 5.7.2
      Kernels: ark, python3

[✓] Checking Jupyter engine render....OK

[✓] Checking R installation...........OK
      Version: 4.4.1
      Path: /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources
      LibPaths:
        - /Users/davis/Library/R/arm64/4.4/library
        - /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/library
      knitr: 1.48
      rmarkdown: 2.27

[✓] Checking Knitr engine render......OK
DavisVaughan commented 1 month ago

Hey look I also reported the highlighting problem in May https://github.com/quarto-dev/quarto/issues/447

This new issue is more severe because the actual code sent to the console is completely broken. Practically this came up in the cpp11 motivations.Rmd (which I am going to patch to avoid this, so don't use that as a test case)

cscheid commented 1 month ago

That syntax isn't valid in Quarto and well, you see, this is a Quarto editing extension, not an RMarkdown editing extension :)

I'm only like 95% joking, too. My understanding is that the extension takes over .rmd files to provide a reasonable experience in RMarkdown documents under Positron, but I'm not sure it's feasible for us to adapt to every .rmd vs .qmd difference faithfully. In the limit, that means recreating the RMarkdown editing experience from RStudio into Positron. That is a valid goal for Positron, but I think it's we should have a long conversation about whether that makes sense for quarto-dev/quarto.

DavisVaughan commented 1 month ago

That syntax isn't valid in Quarto

Is there some technical reason for this that I'm not aware of? It seems like an extremely easy thing to type in a Quarto document. You don't get any feedback about the fact that it's invalid, but it completely borks the document

DavisVaughan commented 1 month ago

Also in https://github.com/quarto-dev/quarto/issues/447#issuecomment-2125090923 you say that the highlighting part is a bug. But the highlighting being off was actually the only way I was able to figure out what was broken!

So if you fix the highlighting but don't fix this, then I would have been completely out of luck!

cscheid commented 1 month ago

Is there some technical reason for this that I'm not aware of?

It's the Markdown variant from Pandoc that we use. FWIW I can't even get VS Code to let me save a file with that syntax - it adds the line break in .md (!)

cscheid commented 1 month ago

https://github.com/user-attachments/assets/555bfc67-b164-463d-ae7e-8445f4206d47

cscheid commented 1 month ago

As I write this, I (predictably) can't make Pandoc dislike that line break. (That is, I no longer can produce direct evidence that this is something we shouldn't support.)

And even if we don't support it, it is something we should add to our quarto-cli planned linter at the very least!

cderv commented 1 month ago

I was just curious to understand two things on this

Is this related to Rmd only ?

So I have tried with a .qmd in Positron and I get the exact same error. So it seems to about executable cell detection in both Rmd or Qmd document.

I pass on the discussion about the syntax. To me this is valid syntax for Rmd and Qmd as we render it correctly when doing quarto render. the intermediate .md passed to Pandoc will have new lines anyway, and Pandoc itself does not need empty line between paragraph and codeblock. Pandoc is unfortunately not consistent on Block parsing - it does require empty line before and after a Fenced Divs with :::.

Is this related to Quarto extension itself, meaning does it reproduce in VSCODE and a R terminal ?

So I tried the same doc in VSCODE, and I can't reproduce. Doing the exact same thing, it sends only x <- 1 in the R terminal when doing control enter.

So it seems related to executable code cell detection in Positron only, which somehow differ from VS Code it seems...

Just wanted to share this if it can help

Code part possibly related to this Not sure yet how to debug VSCODE Quarto extension inside Positron, but we should look into there IMO https://github.com/quarto-dev/quarto/blob/06678d55143a7d6de6c7f0232db0c5dcf8392ca6/apps/vscode/src/providers/cell/commands.ts#L90-L110
DavisVaughan commented 1 month ago

So I tried the same doc in VSCODE, and I can't reproduce

Do you see the bad highlighting in VS Code? Like in my original image above

Update: I do see the bad highlighting in VS Code:

Screenshot 2024-07-26 at 9 32 50 AM

So maybe there really are 2 places this is going wrong.

cderv commented 1 month ago

Yes I see the highlighting problem too. So probably two different problems yes.

juliasilge commented 1 week ago

Another report of folks running into this here: https://github.com/posit-dev/positron/discussions/4531