eddelbuettel / digest

R package to create compact hash digests of R objects
https://eddelbuettel.github.io/digest
111 stars 47 forks source link

The 'raw' argument (digest()) in aes$decrypt was not documented #187

Closed daattali closed 1 year ago

eddelbuettel commented 1 year ago

If find this is described in the man/digest.Rd. Your PR doesn't exactly make clear where to set raw and as such is far from as clear as it could be.

daattali commented 1 year ago

I'm not talking about the digest() function. My original title is correct.

The AES() function returns an object that has a decrypt() function, which takes a raw argument. That argument is currently undocumented. I've added the documentation in the correct place.

Here's a before and after snippet from the documentation of AES():

image

VS

image

eddelbuettel commented 1 year ago

But raw is no argument to AES or am I undercaffeniated?

AES <- function(key, mode=c("ECB", "CBC", "CFB", "CTR"), IV=NULL) {

PS Dang. It is to the decrypt() method. Sorta-kinda a bug in R CMD check to not nag about that. Ah well.

eddelbuettel commented 1 year ago

For simplicity and as we now have two open PRs hitting the same file, how would you feel about cherry-picking this commit into the other open PR?

daattali commented 1 year ago

I thought you would prefer to keep the history clear and have each PR be about exactly one thing, not to combine two different things together. Let me know if you do prefer me to close this and add it to the other PR.

eddelbuettel commented 1 year ago

Fair point but it also just one PR (and to, issue) of "enhancing/updating digest for AES". Plus two competing PRs racing the same file is just ... odd. So if you could a quick cherry-pick of the sole comitt from the other PR (or a merge of its branch or whatever many other methods git has) would be nice.

This all really helps: you identified a weak spot, and brushed it up. That's how it is supposed to work, no? :wink: