FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix language:r hook installation when initiated in RStudio by lorenzwalthert · Pull Request #3389 · pre-commit/pre-commit · GitHub

Fix language:r hook installation when initiated in RStudio - #3389

Merged
asottile merged 1 commit into
pre-commit:mainfrom
lorenzwalthert:dev-always-unset-renv
Jan 20, 2025
Merged

Fix language:r hook installation when initiated in RStudio#3389
asottile merged 1 commit into
pre-commit:mainfrom
lorenzwalthert:dev-always-unset-renv

Conversation

lorenzwalthert commented Jan 19, 2025
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #3385. The bug is that in install_environment(), the env patch was not applied in the following snippet:

    with _r_code_in_tempfile(r_code_inst_environment) as f:
        cmd_output_b(_rscript_exec(), '--vanilla', f, cwd=env_dir)

The existing _execute_vanilla_r_code_as_script() abstracting away

  1. the application of env patch and
  2. the writing inline code to file and executing there (since inline execution lead to parser failures)

was only suitable to execute R code after the environment has been installed and was for that reason not used when the renv environment was set up initially.
Hence, the approach I took was to factor out _execute_r() that does 1. and 2. and can be parametrised as far as CLI options passed to R go, so it can be used to create the environment itself (_execute_vanilla_r()) and also later to run R code in it (_execute_r_in_renv(), previously named _execute_vanilla_r_code_as_script()).

lorenzwalthert marked this pull request as ready for review January 20, 2025 11:18
asottile force-pushed the dev-always-unset-renv branch from ffe1692 to c2c061c Compare January 20, 2025 18:10
otherwise, installing the hooks when RENV_USER env variable is set (e.g. in RStudio with renv project) will result in executing the installation script in the wrong renv

asottile left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

asottile enabled auto-merge January 20, 2025 18:14
asottile merged commit c3125a4 into pre-commit:main Jan 20, 2025
lorenzwalthert deleted the dev-always-unset-renv branch January 21, 2025 11:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

pre-commit does not work when used in a project with R renv

2 participants


Back | FazBrowse Home | New Git URL