pkg_name result retains trailing whitespace from Package field in DESCRIPTION

[kylebaron]

Original context: using pkgdown; there was trailing whitespace in the "Package" field that was causing build_reference to fail.

It looks like the desc package behavior is to preserve whitespace on read:
https://github.com/r-lib/desc/blob/898c0cea313f7df17cadfbc81eb38b9b403c6044/tests/testthat/test-read.R#L13

Then should pkgbuild:::pkg_name trim if it is depending on desc to read the DESCRIPTION file (?)

path <- tempdir()

pkg <- "pkgws"

package.skeleton(name = pkg, code_files = "break.R", path = path)
#> Exiting.
#> Creating directories ...
#> Creating DESCRIPTION ...
#> Creating NAMESPACE ...
#> Creating Read-and-delete-me ...
#> Copying code files ...
#> Making help files ...
#> Done.
#> Further steps are described in '/var/folders/xb/hqmfzgl95fq8mx6tjxkzb71r0000gn/T//RtmpldMIW0/pkgws/Read-and-delete-me'.

x <- read.dcf(file.path(path, pkg, "DESCRIPTION"))

x[1, "Package"] <- "pkgws   "

write.dcf(x, file.path(path, pkg, "DESCRIPTION"), keep.white = "Package")

pkgbuild:::pkg_name(file.path(path, "pkgws"))
#> [1] "pkgws   "

read.dcf(file.path(path, pkg, "DESCRIPTION"))[, "Package"]
#> Package 
#> "pkgws"

Obviously, I dropped the whitespace in my DESCRIPTION file, and everything works now. Reporting for you to consider.

Thanks,
Kyle

[kylebaron]

I'll bring this to pkgload (I think). KTB

[jimhester]

If this is opened anywhere it should probably be desc, it should probably not be preserving whitespace for the package name field

[gaborcsardi]

desc preserves whitespace per policy, although in this case it could give a warning I suppose.
@kylebmetrum would that be sufficient?

[kylebaron]

@jimhester I was originally going to open in desc, but it looked like desc was keeping the whitespace by design and after some consideration.

@gaborcsardi I personally wouldn't give a warning at desc. I thought it was reasonable for desc to return stuff with whitespace, but I'd call on pkg_name to recognize the possibility of white space on the ends and trim as a defensive consideration. The trimmed result seems like the "right" result for pkg_name; I wouldn't really expect users to think about needing to trim that result.

But it's up to you ... I'm only reporting because pkgdown::build_reference() blew up and it wasn't initially obvious why. I have my stuff fixed now. But thinking about all the packages involved it seemed most reasonable to focus on what pkg_name was doing.

Sorry about the confusion ... I just realized this function exists in multiple packages in this ecosystem.

[kylebaron]

Not sure about the details about desc, but maybe desc::desc_get_clean() that trims whitespace when you want to use the result for stuff like pkg_name, forming paths or some application where it is read-only?

[gaborcsardi]

I think this should be definitely fixed. desc keeps the whitespace, but this is really only important, if you are writing the file back to disc. So desc::desc_get() could actually trim it. I don't know of a case when leading or trailing whitespace is possibly meaningful, so why not just do that.

This means that d$set(Package = d$get("Package")) will possibly change DESCRIPTION, because of the trimming, but I think that is OK.

I opened an issue at desc, and will leave this one closed.

Is this OK?

[kylebaron]

Sounds good.