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

docs: make bracketed inline code the default syntax #1334

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mcanouil
Copy link
Contributor

The commits in this pull request update the syntax for inline code in the documentation. The changes include using {r} primarily instead of `r` to denote inline R code. These updates improve consistency in the documentation.

Related to quarto-dev/quarto-cli#10805

@cderv
Copy link
Collaborator

cderv commented Sep 16, 2024

Thanks.

Why do you think using {r} is more consistent ?

It has been asked already in

and I agreed with @cscheid on his answer

the previous syntax is historically very popular and this is an intentional choice.

Though looking into docs and its history, it seems your change in a follow up on

where it seems to have been decided that using `{r}` is the prefered syntax, and the PR missed some place to change the syntax.

As reported in quarto-dev/quarto-cli#10805, the new syntax has a change in default behavior regarding string output compared to knitr and R Markdown context, a difference that could be surprising to existing R Markdown user. I did not know we wanted to push using `{r} instead of `r

I'll defer to @cwickham and @mine-cetinkaya-rundel for their advice on which one to document first for R documents. Especially as I guess a choice has been made for the Quarto book.

If we really go to new syntax as primary one to use, we should probably add an example in Get Started about outputting raw markdown without escaping.

@mcanouil
Copy link
Contributor Author

As reported in quarto-dev/quarto-cli#10805, the new syntax has a change in default behavior regarding string output compared to knitr and R Markdown context, a difference that could be surprising to existing R Markdown user. I did not know we wanted to push using {r} instead of r

That is the question indeed as I really thought the brackets syntax was the one to advertise since the documentation states in several places to use brackets and that the syntax without is still supported.

Regarding the string/markdown, I think there is an issue or discussion about harmonising across engines behaviours.

@cderv
Copy link
Collaborator

cderv commented Sep 16, 2024

Regarding the string/markdown, I think there is an issue or discussion about harmonising across engines behaviours.

Not sure to see which one is it. I am interesting to look at it if you find it;

With {lang} syntax for inline both Jupyter and Knitr engine behave the same, which is different for historical default `r behavior.

Knitr

---
format: html
engine: knitr
---

This will be bold `r "**hello**"`

This won't be bold `{r} "**hello**"`

But now this will `{r} I("**hello**")`

image

Jupyter

---
format: html
engine: jupyter
---

```{python}
from IPython.display import Markdown 
```


This won't be bold `{python} "**hello**"`

But now this will `{python} Markdown("**hello**")`

image

@cscheid
Copy link
Collaborator

cscheid commented Sep 16, 2024

I agree with @cderv's assessment here. I'm willing to eventually replace the "legacy" knitr syntax in our docs, but we need have a broader discussion about it that explains the situation to both knitr and jupyter users. This is not a change I want to make lightly.

@mcanouil
Copy link
Contributor Author

Unfortunately, the change already took place in almost all corner of the documentation. This PR is really a follow up as Christophe highlighted.

@cderv
Copy link
Collaborator

cderv commented Sep 16, 2024

but we need have a broader discussion about it that explains the situation to both knitr and jupyter users. This is not a change I want to make lightly.

Added as Team discussion item for next meeting.

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

Successfully merging this pull request may close these issues.

3 participants