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

Switch R package development support to commands #772

Merged
merged 9 commits into from
Jun 25, 2023

Conversation

juliasilge
Copy link
Contributor

Addresses #747 and also #735

This isn't quite right yet, because the terminal (pseudoterminal?) takes focus while the task is being run.

@juliasilge juliasilge self-assigned this Jun 21, 2023
@juliasilge
Copy link
Contributor Author

Looks like what we want is "reveal": "never".

@juliasilge juliasilge changed the title Start custom execution for R pkg tasks Switch R package development support to commands Jun 22, 2023
@juliasilge
Copy link
Contributor Author

I know I was the one making the argument that these package management needs should be tasks, but the API is much more complicated compared to commands and I now think the best option in the short term is to offer these as commands.

In the long run, we can reevaluate whether tasks are better, because they would allow us to take advantage of the problem matchers infrastructure.

@juliasilge
Copy link
Contributor Author

juliasilge commented Jun 22, 2023

Should I add some quick-and-dirty keybindings for these commands here, for the private alpha? Or wait for #669? It would look like:

    "keybindings" : [
      {
        "command": "r.packageLoad",
        "key": "ctrl+shift+l",
        "mac": "cmd+shift+l",
        "when": "isRPackage"
      },
      {
        "command": "r.packageBuild",
        "key": "ctrl+shift+b",
        "mac": "cmd+shift+b",
        "when": "isRPackage"
      },
      {
        "command": "r.packageTest",
        "key": "ctrl+shift+t",
        "mac": "cmd+shift+t",
        "when": "isRPackage"
      },
      {
        "command": "r.packageCheck",
        "key": "ctrl+shift+e",
        "mac": "cmd+shift+e",
        "when": "isRPackage"
      }
    ]

Also, I can quickly add devtools::document() too

@jmcphers
Copy link
Collaborator

I think these keybindings are already taken in VS Code so we shouldn't override them with RStudio keybindings unless the user has intentionally enabled RStudio keybindings.

Maybe the default keybindings for these could use some kind of leader sequence? (e.g. VS Code uses Cmd+K a lot as a leader sequence)

@juliasilge
Copy link
Contributor Author

juliasilge commented Jun 22, 2023

Totally happy to wait on the keybindings but with the when clause, it only overrides them when we are in an R package (and if the user has their own custom keybindings, those take preference). I am totally fine to wait on this until we have a more comprehensive keybindings solution, but I can't think what else someone would want to use, say, a default build task key binding for in an R package, other than this.

@jmcphers
Copy link
Collaborator

Agree, we could probably take over the Run Build Task keybinding. The others, though, already have meaning. Cmd+Shift+E focuses the Explorer tab, Cmd+Shift+T focuses the previous editor, etc.

@juliasilge
Copy link
Contributor Author

OK, let's wait on the keybindings then. 👍

@juliasilge juliasilge marked this pull request as ready for review June 22, 2023 21:50
@juliasilge
Copy link
Contributor Author

Addresses #28

#122 is about about stopgap keybindings for an initial release, so if we don't want to do that in favor of a better longer term option, maybe we should close?

@jmcphers
Copy link
Collaborator

Yeah, I think we should close #122.

@jjallaire
Copy link
Contributor

I actually think that we should bind these keyboard shortcuts, because they are scoped to only exist when the user is working on an R package (a strong signal that they'll use and benefit from these keybindings). A similar example was that I took Cmd+Shift+K for Render in Quarto (even though it is "delete line" in VS Code). If these are bound to mainline, commonly used editing commands that's one thing, but if they are more obscure I think we should take them.

@juliasilge juliasilge requested a review from jmcphers June 23, 2023 17:50
@juliasilge
Copy link
Contributor Author

Now definitely addresses #122 with our initial solution (to be replaced in the longer term by #669)

@jmcphers jmcphers merged commit c0011d7 into main Jun 25, 2023
@wesm wesm deleted the r-pkg-tasks-custom-execution branch March 15, 2024 18:29
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