search

juba / robservable

Observable notebooks as R htmlwidgets
https://juba.github.io/robservable/
163 stars 11 forks source link

name for `cell` argument #25

Closed timelyportfolio closed 4 years ago

timelyportfolio commented 4 years ago

In #23 we started to discuss whether the cell argument should be something else. I think cell is fine, but that is likely because I use Observablehq quite a bit, and the term is familiar to me. We could opt for a verb similar to hide. I included a couple of options:

I do not have a strong opinion. If we change, we will need to make sure we change the examples.

juba commented 4 years ago

I agree with the idea to use a verb, seems more natural and coherent with hide.

I've got a small doubt about show, because there are cases when we want to include a cell without showing it (in this case the cell would be both in show and hide, which feels a little strange). Both include and render seem good to me... Maybe include is more "natural" ?

timelyportfolio commented 4 years ago

good point on show and I agree include seems like a good choice. render I think carries much closer association with show. Will try to think of a couple of others (hopefully shorter) verbs. I guess add might be an option. Both include and add are not in base R so should be safe there even though it doesn't really matter if we default to NULL.

timelyportfolio commented 4 years ago

join, bind, link, connect

juba commented 4 years ago

Thanks for your propositions ! I think I've got a small preference for include. add is shorter, but it seems to me that by conveying the idea of adding something it is not totally coherent with the fact that it is rather "selects" specific cells...

But maybe you find that include is too long ?

I'm less convinced by join, bind, link and connect, each one seems to carry a slightly different meaning...

timelyportfolio commented 4 years ago

@juba include works for me. Want me to make the changes in function, JS, documentation, and examples?

juba commented 4 years ago

If you have the time and are ok to do it, yes !

I can do it in the vignettes if you want to share the "burden".

timelyportfolio commented 4 years ago

@juba I'm happy to change all as long as next day or so is an acceptable time for completion.

timelyportfolio commented 4 years ago

@juba I hope I changed all cell to include. I tested all examples, vignettes, and rmarkdown that I found in the repo, and all seemed to work. Since this is breaking I incremented to 0.2.0 and updated package date. Please check if you have time that I successfully rid everything of cell =. Thanks.

timelyportfolio commented 4 years ago

@juba sorry I messed up with origin and upstream and ended up needing to push to master. Hope this doesn't pose any problems.

juba commented 4 years ago

Thanks a lot !