r-lib / desc

Manipulate DESCRIPTION files
https://desc.r-lib.org/
Other
122 stars 27 forks source link

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

Open ateucher opened 1 year ago

ateucher commented 1 year ago

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 commented 1 year ago

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

Yes, please!

ateucher commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.