return empty data.frame not NULL in renv_available_packages_success #1774
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This avoids downstream errors in `row()` where `nrow(NULL)` returns `NULL`
|
Thanks! |
zkamvar
added a commit
to joelnitta/sandpaper
that referenced
this pull request
Dec 6, 2023
milanmlft
pushed a commit
to LearnToDiscover/sandpaper
that referenced
this pull request
Dec 12, 2023
* document
apparently last commit did not run document(), this adds Milan Malfait
who was added in DESCRIPTION as ctb
* Add scaffolding to translate with PO files
based on {pkgdown} scheme and using {potools}
* Flag strings for translation
* Add POT file
* Add Japanese translations
* Add test for translations
* Use local_envvar_pkgdown() instead of section_init()
Co-authored-by: Zhian N. Kamvar <[email protected]>
* Delete un-needed functions
Co-authored-by: Zhian N. Kamvar <[email protected]>
* Remove superfluous line
Co-authored-by: Zhian N. Kamvar <[email protected]>
* Delete un-needed function
* Insert translations for mustache elements
* Add JA translations
* allow translated vars to be included in site data
The reason why the initial apporach I proposed was not working is
because this bumps into a few concepts that have not been clearly
documented on this end.
First, there is the concept of the `pkg` object. This object is a
representation of the _pkgdown.yaml file from the `site/` folder. When
any page is built, this is passed to `pkgdown::data_template()`, which
creates the data needed for the whisker templates (e.g. everything in
the `$params` section of the `_pkgdown.yaml` file is shunted into `$yaml`)
and boilerplate items like dropdown menu items (for pkgdown) are added.
In my original conception, I would be able to add `$translate` to the
`pkg` object and it would be propogated to the website. This was not
true and this is where the disconnect initially lies. The problem was that the
code for `pkgdown::data_template()` was _replacing_ the `$translate`
field with the default translations from {pkgdown}.
In this package, the way we pass variables to {varnish} via the
`_pkgdown.yaml` file, the generated content for `learner_globals()` and
`instructor_globals()`, and the `this_metadata()` object, which contains
both supplied and generated metadata content.
All of these are passed to `build_html()`, which uses
`pkgdown::render_page_html()`, passing the generated global data to the
`data` parameter and _this_ is where the translations are incorporated.
* fix typo
* add translation terms that I found in varnish
* fix typo
* fix more typos
* apply variables to translation strings
* begin data flow vignette
* add information about varnish content
* begin work on fine-tuning translated strings
* clean up varnish terms a bit
* re-run potools::po_extract()
* run potools::po_update() and fix fuzzy matches
* update variable names
* add lang attribute to pkgdown template
* add fill_translation_vars function to glue data
This will run before pkgdown is run so that we don't have to worry about
syntax or messing up HTML. It's not the perfect solution, but it will
work.
* Add JA translations
* fix license text with hyperlink
* organising a bit more...
* use for-loop for my gluing
* use explainer variable
* add mechanism to include HTML templating
There are _many_ situations where we want to translate something that
has a link _inside_ of the translation itself. It's rather awkward to
force the translator to translate something like this while keeping the
HTML:
Please visit <a href="CODE_OF_CONDUCT.html">the Code of Conduct</a>
Because we know what these HTML elements are supposed to be, we can get
away with this by using <( and )> for the start and end pieces of HTML
that we wrap.
* rerun potools suite and fix fuzzy messages in ja
* compile ja translations to confirm it works.
* fixed index sidebar item; add documentation
It turned out that we were initialising the sidebar before `build_html()`
was called, so I had to add a translation mechanism. Not the best, but
it will work.
* fix complex BuiltWith string and template license
* fix url templating for specific case
* rerender po files (there is still one fuzzy)
* Update JA translations
* update license translation string; anchor string
* add translation for Overview;Objectives;Questions
* collect, update, and compile messages
* add documentation for the lang config item
* add Joel as an author and translator
* document
* add translate_callout_heading()
* update translations
* bump version to dev; add news
* Update JA translations
* add test for code blocks
* update fix_codeblocks to include translations
* add translations for output blocks
* fix small bug in apply_translations
* add translations that we will implement in future
* collect translations; add my attempt for SpanToTop
The Back To Top link is split out so that the word "Back" is encased in
a `<span>` element that disappears on smaller screens so that the
message becomes "To Top".
This update translates `<(Back)> To Top` in a way that allows the translator to
choose which text should disappear on smaller screens. In languages with
SVO syntax (as English is), the order will stay the same and the word
for "Back" will be on the top, but for languages with SOV syntax
(Japanese, Korean, German), it will appear as `To Top <(Back)>`. I've
attempted to do this for the Japanese translation, but it probably
should be corrected.
I'm not sure how flexible this is, but this is an attempt.
* compile translations
* update and recompile translations for JS
* add known_languages() function and `Toggle Menu`
* add test for aria menu items
* update pofiles
* update DECRIPTION and NEWS with withr
* begin work on vignette
* add translations vignette to pkgdown
* add another paragraph
* fix breaking tests
* ensure the translation vars are always set
We default to english here if the variable is not set.
* document and add more stuff to vignette
The vignette is trash
* update vignette to be more informative
* update pkgdown
* Make elements of images section translatable
* Add JA translations
* use normal glue braces for translation templating
I had overengineered once again and this fixes that.
* update vignette
* skip when we are on lower versions of windows
* update NEWS
* make it explicitly less than 4.2
* move things around a bit
* skip {renv} test on macOS
This appears to be transient, but it's really difficult to pin down the
issue when it is coming from an external source. It is better just to
skip this on macOS than it is to pin down the problem.
* trace the generation of metadata
This will get stale, but I'm not sure of a better way of doing it
:grimace:
* skip actual weird test on mac
* add more things to vignette
* move translating functions into utils-translate
* apply further tests for translations
* create code to fix the accordions
* add xml_text_translate(); use for accordions
* replace translate_callout_heading
This replaces it with a more efficient method by translating once and
then applying those translations over the nodes. This is still not
efficient as can be. The translations _could_ live in yet another global
environment that can be initialized when the config file is read in, but
we will optimize that later
* extract and update translations
* no need to skip on macos anymore
see rstudio/renv#1774
* describe function factories a bit
* Update JA translations
* add more detail about specific data sources
* add some fuzzy details about pkgdown
* fix booboo
* rearrange and clarify a bit
* add note
* remove skipper
* change `local_envvar_pkgdown()` to `set_language()`
This function does not need to rely on the pkgdown structure. We have
the language recorded in the metadata, so that should be sufficient.
* add catch to not set unknown language
* add some spanish translations
I retrieved some of them from carpentries/styles-es, but I could not
translate all of them.
* Update DESCRIPTION to use main branch of varnish
* Translations R-es.po
I did the remaining translations to Spanish. Thanks so much, @zkamvar and @joelnitta, for building this.
Thanks a lot, @acrall, for working so hard in advance in the localization of The Carpentries materials.
* Recompile Español translations.
potools::po_compile()
* Add @yabellini as a translator; update NEWS
---------
Co-authored-by: joelnitta <[email protected]>
Co-authored-by: Zhian N. Kamvar <[email protected]>
Co-authored-by: Yanina Bellini Saibene <[email protected]>
Co-authored-by: Erin Becker <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses a bug introduced in the fix for #1706.
On 2023-12-05 at 09:21 PST, I had a successful check for MacOS to ensure that a package added with
renv::record("pkg@version")would be successfully recorded.EDIT: for context, this test has been running successfully for several months (see carpentries/sandpaper@0de3543 for the last time this particular code was modified)
The very next check on 2023-12-05 at 11:07 PST, had a failure in MacOS
The relevant portion of the error log reads:
This code is found in the
rows()function:renv/R/utils.R
Lines 430 to 437 in 4c24748
When
nrow(NULL)is called, it returns a NULL and then give the error that I see. I believe this was caused by d872238, where it returns a NULL instead of an empty data frame.