default values

[MikeBadescu]

Thank you for the great package!

Would it be possible to specify default values in the formals of functions like show(), hide() and toggle? The default values are mentioned in the documentation but not in the source code. Similarly, selector in enable(), disable() could be NULL by default.

This is mostly an annoyance since RStudio is showing Code Diagnostic warnings such as argument 'id' is missing, with no default.

[daattali]

When exactly do you see this error? The default values are on the
Javascript by the way
On Aug 25, 2016 09:45, "Mike Badescu" [email protected] wrote:

Thank you for the great package!

Would it be possible to specify default values in the formals of functions
like show(), hide() and toggle? The default values are mentioned in the
documentation but not in the source code. Similarly, selector in enable(),
disable() could be NULL by default.

This is mostly an annoyance since RStudio is showing Code Diagnostic
warnings such as argument 'id' is missing, with no default.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#80, or mute the thread
https://github.com/notifications/unsubscribe-auth/AA6IFOPcqq5STaERiKtN1_HpuRZgi_Vrks5qjcbCgaJpZM4JtRkr
.

[MikeBadescu]

This is what I see in RStudio 0.99.1293 (but for many previous versions as well), R 3.3.1 x64 on Windows 10.

Edit: all Code Diagnostics enabled in RStudio

[daattali]

I do see that now if I enable all the code diagnostics. I find that quite annoying, but more so on RStudio's part. I wouldn't consider this a problem with shinyjs personally. I don't think it's bad practice to have arguments to a function and not give them default values. I would strongly argue that this should not be a "warning" at all, it's a very normal thing to have missing arguments. But maybe the RStudio guys believe otherwise, in which case I'll consider adding defaults.

[MikeBadescu]

My (subjective) view is that having no default value for an argument implies that the argument must be supplied. Also, style guidelines (e.g. spacing) are not required in R, but they make the code readable. Thank you for considering this.

[daattali]

I do see your point, and I will agree with you in general, but the shinyjs functions have all their default values set in the javascript. Initially I just had "..." as the arguments to the functions because all the args are just passed directly into javascript and it's important not to touch them in any way before, and I just documented in the docs what the actual arguments are. But later on I decided it's much better for users to be able to see exactly what arguments are used, so I added the arguments explicitly. But I don't want to give them defaults in R because then they'll have duplicate default values in two places and that feels very wrong, and simply passing along all the params seems like the more correct solution. If javascript was not involved and the entire function was just implemented in R then I'd agree with you, default values would make more sense. And I don't want to just use NULL as default values because in some cases NULL is actually meaningful but sometimes it isn't.

[MikeBadescu]

I have run into the double defaults issue, it helps to have only one source of defaults, either in the upstream function or in a helper function / dictionary. I am not sure how it would work with JS (not a JS expert). It would be great if it were possible.

(speculating) Maybe the string 'null' could be a default for passing NULL.

Also, thank you for taking your time and asking the community.

[daattali]

You can see how the default values are assigned in JS: here and here as an example. The nice thing is that I don't have to worry about what params are passed in and which aren't - JavaScript has a nice "extend" function that deals with that. I really can't think of any way to share common default value between both R and JS... Oh, and 'null' is actually equivalent to NULL in JS so that specifically is not a great solution :p

[daattali]

FYI I'm starting to agree with you and think that default values should be provided, but for different reasons. The rstudio warnings are a bit annoying but they shouldn't rule how I write my code 📦

My reason is that I think for the end user it's more convenient to see the default values when they autocomplete the function rather than having to read the documentation to see what they are. I might do that for the next version.

[MikeBadescu]

Good point. But it will affect many functions ... either use new function names and deprecation warnings or go for shinyjs2 😄

[daattali]

I will use the same defaults that currently exist, the only difference is that they will be defined in R instead of in JS. It shouldn't break anybody's code

…

On Mar 17, 2017 10:34 AM, "Mike Badescu" ***@***.***> wrote: Good point. But it will affect many functions ... either use new function names and deprecation warnings or go for shinyjs2 😄 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#80 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA6IFB7QKRIghPSHMKfbRcdoDur2yiWvks5rmpnegaJpZM4JtRkr> .

[daattali]

@MikeBadescu I finally got around to doing this.

I'm very scared something may have broken. If you have a chance at some point, I'd appreciate if you could install from github and make sure your apps still work

[MikeBadescu]

Dean, thank you!

I installed from GitHub and tested it - so far no issues.

[daattali]

Awesome. I've done a lot of changes so you might run into some issue somewhere:)

…

On Nov 12, 2017 8:29 AM, "Mike Badescu" ***@***.***> wrote: Dean, thank you! I installed from GitHub and tested it - so far no issues. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#80 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA6IFI-5mN8xE3kBP10DsxmjRWrLluTCks5s1vLDgaJpZM4JtRkr> .