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

Setting default = "" in get_list() for a non-existent field returns empty character vector #138

Open
ateucher opened this issue May 3, 2023 · 4 comments
Labels
documentation tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day

Comments

@ateucher
Copy link
Contributor

ateucher commented May 3, 2023

I believe this should return the default value set "":

library(desc)
desc <- desc(text = "Package: mypkg")

desc$get_list("foo", default = "")
#> character(0)

I believe the culprit is here, calling strpslit() on the empty string returns character(0).

I'm happy to do a PR if you'd like

Created on 2023-05-03 with reprex v2.0.2

@gaborcsardi
Copy link
Member

I'm happy to do a PR if you'd like

Yes, please!

@gaborcsardi gaborcsardi added the bug an unexpected problem or unintended behavior label May 3, 2023
@ateucher
Copy link
Contributor Author

ateucher commented May 3, 2023

Ok great. Is the intention that default should be a length 1 character vector (including ""), but nothing else? Or should it be allowed to have length > 1? I think default is meant to be returned verbatim, so it should allow length >=1...

@gaborcsardi gaborcsardi added documentation and removed bug an unexpected problem or unintended behavior labels May 4, 2023
@gaborcsardi
Copy link
Member

Actually, now that I took a better look at this, I don't think that it is a bug. The default value is not value that is should be returned, but the value that we pretend to be in the config file. E.g.:

❯ desc::desc_get_list("foo", default = "list, item, three")
[1] "list"  "item"  "three"

❯ desc::desc_get_list("foo", default = "")
character(0)

AFAICT usethis also uses it this way: https://github.com/cran/usethis/blob/cee86ced02745b9552de72355b5d4152472fbfe8/R/description.R#L206

The documentation could be improved, though.

@ateucher
Copy link
Contributor Author

ateucher commented May 4, 2023

Yeah, that's actually where I discovered it. I was working on r-lib/usethis#1837 (which replaces the code you linked to above) and was just surprised by the character(0) returned when looking for a non-existent key here.

But if that's the expected value, then I'm happy with that.

@gaborcsardi gaborcsardi added the tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day label Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants