Skip to content

Conversation

@ben-schwen
Copy link
Member

@ben-schwen ben-schwen commented Jan 26, 2026

Follow up to #7534

Currently the translation links are only correctled rendered and displayed for the english vignettes

CI output

@ben-schwen ben-schwen requested review from a team and MichaelChirico as code owners January 26, 2026 07:23
@ben-schwen ben-schwen requested review from aitap and removed request for a team and MichaelChirico January 26, 2026 07:37
@ben-schwen
Copy link
Member Author

I just noticed that we can test this locally with litedown::fuse('vignettes/ru/datatable-intro.Rmd') which ensures that render_translations works correctly and R CMD build . which ensure that the translation links for the english vignette are built correctly.

@aitap
Copy link
Member

aitap commented Jan 26, 2026

Looks like the fix in cd98747 was necessary, because when the vignette is rendered, the current directory is vignettes, not the package source root directory, so dirname(context) didn't match vignettes, at least on my PC and in CI. And the rest of the script works after the fix thanks to the dir(path) call not setting full.names = TRUE, so dirname("foo.Rmd") is still ..

The fread vignette and its translations were also using the rmarkdown syntax for inline code blocks (`r ...` instead of `{r} ...`).

So now it should be fine, right?

@ben-schwen
Copy link
Member Author

When running locally I now get due to cd98747

litedown::fuse('vignettes/ru/datatable-intro.Rmd')
# Warning message:                                                   
# In normalizePath(dirname(context)) :
#   path[1]="vignettes/ru": No such file or directory

Copy link
Member

@aitap aitap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing the issues I had missed!

@aitap aitap self-requested a review January 26, 2026 14:23
aitap added 2 commits January 26, 2026 17:28
Just check against "vignettes/foo.Rmd" or (implied ./)"bar.Rmd".
@aitap
Copy link
Member

aitap commented Jan 26, 2026

  • R CMD build ., R CMD INSTALL, browseVignettes(): links exist and point where they are supposed to
  • Rscript site/render_translations.R: no more warnings, links also seem fine

I think it's ready to be merged.

@ben-schwen
Copy link
Member Author

Yeah, I also just checked locally and everything looks fine. TY!

@ben-schwen ben-schwen merged commit ae1cf7c into master Jan 26, 2026
8 checks passed
@ben-schwen ben-schwen deleted the vignette_output_format branch January 26, 2026 15:05
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.

2 participants