Fix bug

[maelle]

As suggested by @bcheggeseth

[maelle]

It also seems

mailmerge/R/mail_merge.R

Line 84 in 86dc0c0

if(!preview && !confirm) {

should be if(!preview && confirm) I think?

I was also wondering whether the default value of confirm should be rlang::is_interactive().

Lastly, I think it'd make sense to add a test that'd have caught the bug (i.e. with confirmation, and send equal to e.g. draft). If not with stubbing I can recommend https://books.ropensci.org/http-testing/ 😁

Thanks again for your work on this useful package @andrie! (and thanks @bcheggeseth for the bug fix).

[andrie]

should be if(!preview && confirm) I think?
I was also wondering whether the default value of confirm should be rlang::is_interactive().

No, I think the current logic is correct, since confirm is defined as :

If FALSE asks for confirmation before sending.

Also, I don't think the default should depend on interactivity. IMO it's safer when using this in production (i.e. non-interactively) to fail if confirm == FALSE.

[andrie]

Lastly, I think it'd make sense to add a test that'd have caught the bug (i.e. with confirmation, and send equal to e.g. draft). If not with stubbing I can recommend https://books.ropensci.org/http-testing/ 😁

Yes, this makes sense. If I can add a test in less than 30 minutes I'll add it to the package, otherwise leave this part for a future date.

[andrie]

Thank you :-)

[andrie]

I've submitted a new version to CRAN.

Also note that the new default branch is main (renamed from master)