Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update keyboard shortcut to insert native pipe #786

Closed
wants to merge 1 commit into from

Conversation

statik
Copy link

@statik statik commented May 18, 2023

This change updates the keyboard shortcuts in interactive tutorials to insert the native pipe instead of magrittr pipe.

If you would like this to be a configurable option, I'm happy to do more work to make it configurable - it wasn't obvious to me how to make this configurable at first.

I got some failures from devtools::check() but doubt those are related to my changes, I am unsure if I have my local dev environment correctly configured.

PR task list:

  • Update NEWS
  • Add tests (if possible)
  • Update documentation with devtools::document()

Signed-off-by: Elliot Murphy <statik@users.noreply.github.com>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@gadenbuie
Copy link
Member

This should definitely be configurable, but we currently don't have much infrastructure for setting or communicating the preference.

Here's a quick brain dump of an idea:

  • We could use |> as the default but have the Shiny server report the R version on startup. If using R < 4.1, we could update the shortcut to use %>%. This would be handled via session$sendCustomMessage('learnr-r-version', R.version) in the R side and a custom message handler on the client side (set via Shiny.addCustomMessageHandler('learnr-r-version', function(data) { ... }).

  • We could then make this configurable in the YAML header of the tutorial. (I'd be tempted to have it be an argument to tutorial(), but then we'd also want that argument to cover other shortcuts.) When rendering the tutorial, we'd add a Shiny pre-rendered chunk to the document that would encode this option, or choose based on R version.

    So instead of sending the R version to the client, we'd read it on the R side and decide there which shortcut to use. But the communication pathway would be the same.

@gadenbuie
Copy link
Member

@statik thanks for this PR! I ended up going in a slightly different direction in #804 that allows the pipe to be set at the level of a tutorial or even at the level of a specific exercise. Let me know what you think!

@gadenbuie gadenbuie closed this Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants