search

fmmattioni / downloadthis

Implement Download Buttons in rmarkdown 💾
https://fmmattioni.github.io/downloadthis/
Other
147 stars 5 forks source link

Optional output_name #6

Closed JohnCoene closed 4 years ago

JohnCoene commented 4 years ago

Hi Felipe,

Really neat package you put together!

Below are the changes included in the PR, so are good practice (I hope) others are rather opinionated.

  1. Move the match.arg at the top of download_this, before said arguments are used otherwise function can still fail.
  2. Use inherits instead of class since the latter can return a vector of length > 1 and thus may break the if statement check
  3. Simplify an error message, I think you (or your function) is too polite, no need to apologise in there :smile:
  4. Make the output_name optional. The function deparses the data object to obtain a string download_this(mtcars) will download a file mtcars.csv.
  5. Ensure the tmp file is deleted when the function exits

Let me know what you think :)

fmmattioni commented 4 years ago

Hi @JohnCoene!

Thank you so much for the PR! This looks fantastic! Thanks a lot for all the improvements! I never used inherits before, it is good to know that is good practice!