-
Notifications
You must be signed in to change notification settings - Fork 320
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: add limitations related to downlit
#6297
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
I added some comments because I don't think that the addition really clarifies the situations.
Basically the current limitation due to downlit procesing is that
code-link: true
has no effect when code-line-numbers
or code-annotations
features are used.
But both code-line-numbers
and code-annotations
features works well. Do you have an example where they don't work ?
This is a limitation for code-linking features which uses R packages and this is for now added to
https://quarto.org/docs/output-formats/html-code.html#code-linking
We could also add mention of code-annotations
impact in there IMO.
Not valid for all formats, only HTML. Also, it's `code-link` only that won't work.
As suggested, I made a companion PR: |
For `code-link` to work properly, `code-line-numbers` and `code-annotations` | ||
require to be set to `false`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know if you tried but slight side effect.... setting code-annotations: false
does also prevent code linking it seems if the annotations are present in the chunk.
Basically, if one wants code linking in a cells, it should not contains any annotation using # <1>
syntax.
I don't know why code-annotations: false
make code linking not work because I would have thought the HTML would not be changed in that case. (which is the underlying issue with downlit)
I would say that the advice would be
- For
code-link
to work properly,code-line-numbers
should not be activated on the cell and No code annotations syntax added.
But again, probably a bug that code-annotations
still do something... unless this is the # <1>
syntax that makes downlit fails... 🤔
Are you observing like me ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, I made a repository to check various combinations (but only when defining the option in the yaml frontmatter): https://github.com/mcanouil/quarto--code-link--code-annotations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, for code-line-numbers
, see https://github.com/mcanouil/quarto--code-link--code-line-numbers.
This indeed prevents code linking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really weird - I can indeed reproduce and see that code links works in your example, but I maintain that in my previous example it still does not. Did you try ? #6297 (comment)
Simplified example
---
format:
html:
code-link: true
code-annotations: true
---
```r
stats::rnorm(1) # <1>
stats::rnorm(2) # <2>
```
1. Test 1
2. Test 2
So we should definitely try every possible case and check how that works - for now I don't understand why this would work in the above and not in your case. Looking into it.
Thanks for the repo by the way !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry it has been a while. Do we need to check more cases for theses options before merging ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to say, indeed it's been a while and I did not update https://github.com/mcanouil/quarto--code-link--code-line-numbers with your last example.
I'll have to add those tests and check the results.
This PR adds limitations notes with the use of
downlit
viacode-link: true
.Fixes #6199