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

Allow for use of styler via python pre-commit without R package {precommit} #662

Closed
stefanoborini opened this issue Aug 18, 2020 · 15 comments

Comments

@stefanoborini
Copy link

If I want to use styler and only the styler hooks, I don't want to be forced to install precommit, which includes a lot of other stuff. I should be able to have plain styler and add only its hooks.

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Aug 18, 2020

Hi @stefanoborini,
If you want to use the styler hook, as implemented in https://github.com/lorenzwalthert/precommit, you need to install the pre-commit executable via the various installation options. You don't need to install the R package {precommit} as decribed in the FAQ. Note that you don't need conda anymore. You also don't need to use other hooks from https://github.com/lorenzwalthert/precommit. But you can't use the styler hook without the pre-commit executable. Why don't you owant to install the framework? You can also put your bash scripts with calls to styler in .git/hooks but then you get all the problems with portability, consistency, file filtering etc. for which pre-commit was created, documented and tested for.

@stefanoborini
Copy link
Author

@lorenzwalthert What I mean is that there's no pre-commit-hooks file definition in styler. To get the hooks, you need to install the precommit R package.

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Aug 19, 2020

What I mean is that there's no pre-commit-hooks file definition in styler

You mean in this GitHub repo? Can you be more specific about which file you are refering to? The hook definition is hosted in https://github.com/lorenzwalthert/precommit, in the file .pre-commit-hooks.yaml, which link to files in inst/bin. This git repo only contains a config file on which hooks to use (.pre-commit-config.yaml) when you want to commit to styler.
Even if this repo had a .pre-commit-hooks.yaml file and defined the hook here, you'd need to nevertheless install pre-commit (the upstream framework). You would not need to install the R package {precommit} (you also don't have to with the current way it is working for the styler hook). Here is some documentation on it: https://lorenzwalthert.github.io/precommit/articles/FAQ.html

@stefanoborini
Copy link
Author

You mean in this GitHub repo? Can you be more specific about which file you are refering to? The hook definition is hosted in https://github.com/lorenzwalthert/precommit, in the file .pre-commit-hooks.yaml, which link to files in inst/bin. This git repo only contains a config file on which hooks to use (.pre-commit-config.yaml) when you want to commit to styler.

I don't want to install precommit R. It brings in a lot of other dependencies I am not willing to provide in my environment, and in fact it does not work if I don't have some packages. I want just to use styler with pre-commit (the python utility)

Even if this repo had a .pre-commit-hooks.yaml file and defined the hook here, you'd need to nevertheless install pre-commit (the upstream framework)

Why would I need to add things I am not interested in? I just want to use styler.

You would not need to install the R package {precommit} (you also don't have to with the current way it is working for the styler hook). Here is some documentation on it: https://lorenzwalthert.github.io/precommit/articles/FAQ.html

the hooks in pre-commit-hooks on precommit (r package) are here

https://github.com/lorenzwalthert/precommit/blob/master/.pre-commit-hooks.yaml

They provide roxygenize, styler, and many other things. I want to use only styler, and I don't want to install precommit (r package) in my environment to either deploy the hooks. In fact, when I tried, I got an error about docopt missing

repos:
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v2.4.0
    hooks:
    -   id: trailing-whitespace
    -   id: end-of-file-fixer
    -   id: check-added-large-files
    -   id: check-toml
-   repo: https://github.com/lorenzwalthert/precommit
    rev: v0.1.2
    hooks:
    -   id: style-files
$ pre-commit run -a
<snip>
style-files..............................................................Failed
- hook id: style-files
- exit code: 1

Error in loadNamespace(name) : there is no package called ‘docopt’
Calls: :: ... loadNamespace -> withRestarts -> withOneRestart -> doWithOneRestart
Execution halted

The reason is probably that I don't have docopt in my environment, and I don't want it. I have no idea what is using docopt, but styler is definitely not using it.

  - styler (1.3.2)
    - backports [1.1.8]
    - cli [2.0.2]
    - magrittr [1.5]
    - purrr [0.3.4]
    - R.cache (0.14.0)
      - R.methodsS3 (1.8.0)
      - R.oo (1.23.0)
        - R.methodsS3 [1.8.0]
      - R.utils (2.9.2)
        - R.oo [1.23.0]
        - R.methodsS3 [1.8.0]
      - digest [0.6.25]
    - rematch2 [2.1.2]
    - rlang [0.4.7]
    - rprojroot [1.3-2]
    - tibble [3.0.3]
    - withr [2.2.0]
    - xfun [0.16]

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Aug 19, 2020

the hooks in pre-commit-hooks on precommit (r package) are here

I know, I created this (and the majority of styler).

I want just to use styler with pre-commit (the python utility)

Then install the python utility with one of the installation methods described in the README of lorenzwalthert/precommit and add the styler hook to your config file. Then, in the .pre-commit-config.yaml, put this:

-   repo: https://github.com/lorenzwalthert/precommit
    rev: v0.1.2
    hooks:
    -   id: style-files

Then run precommit install in your repo and you should be good to go.

In fact, when I tried, I got an error about docopt missing

Because in your config, there are all the hooks that you said you don't want to use 😄

@lorenzwalthert lorenzwalthert changed the title Allow for use of styler without precommit in pre-commit Allow for use of styler via python pre-commit without R package {precommit} Aug 19, 2020
@stefanoborini
Copy link
Author

stefanoborini commented Aug 20, 2020

I want just to use styler with pre-commit (the python utility)

Then install the python utility with one of the installation methods described in the README of lorenzwalthert/precommit and add the styler hook to your config file. Then, in the .pre-commit-config.yaml, put this:

This is exactly what I have done.

In fact, when I tried, I got an error about docopt missing
Because in your config, there are all the hooks that you said you don't want to use 😄

And that's my point. Why do I have to install docopt as a dependency in my R environment for hooks that come from precommit (r package) which I am not going to use? If I need to only use styler, I need a hook in the styler git repo, point to that repo, install styler in my R environment, and have styler do its job, without any need for docopt.

My config is literally the one I gave above. It only contains the style-files id. Nothing else.

@stefanoborini
Copy link
Author

Just for curiosity, what is the entity that is requiring docopt in order to run?

@lorenzwalthert
Copy link
Collaborator

I am sorry but I was wrong, docopt is used in the styler hook to parse command line options. It's the only dependency you need here and it has 0 non-base reverse dependencies.

@lorenzwalthert
Copy link
Collaborator

I think we can close this, feel free to re-open if necessary.

@stefanoborini
Copy link
Author

@lorenzwalthert Depends. Is it fixed? I still would prefer not to install a secondary package that has nothing to do with it. I see no logical reason to do so.

@lorenzwalthert
Copy link
Collaborator

I don't plan remove the docopt dependency if that's the question.

@stefanoborini
Copy link
Author

stefanoborini commented Sep 1, 2020

No, that's not my question. docopt is a dependency of styler (but if that's the case, it should probably be advertised in the DESCRIPTION file, as it is not at the moment). My point is that I don't want to install the precommit R package so that I can use styler with pre-commit.

@lorenzwalthert

@lorenzwalthert
Copy link
Collaborator

docopt is not a direct dependency of styler. You can see it in the DESCRIPTION file of styler. A package that does not list a dependency in DESCRIPTION will fail R CMD check and therefore won't ever be released on CRAN. As pointed out above, docopt is a dependency of the hook script the python pre-commit framework will run when you invoke git commit in a repo that has pre-commit activated. This hook script will be run no matter you install the R package {precommit} or not.

@stefanoborini
Copy link
Author

stefanoborini commented Sep 1, 2020

I understand. You should move the hook script here and put the dependency docopt here. That is the whole point of the initial issue. docopt slipped in during the discussion.

It is pointless to install a separate package (precommit, the R package) to use this one!

@lorenzwalthert

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Sep 1, 2020

It is pointless to install a separate package (precommit, the R package) to use this one!

As previously discussed, you don't need to install the R package {precommit} to run the hook. You need all the packages installed globally that are required by the hook script, which for the styler hook is docopt and styler. There is no dependency management for R hooks because R is not (yet) an officially supported language in the pre-commit framework so you need to install them manually as descrbied in https://github.com/lorenzwalthert/precommit#caution.

You should move the hook script here and put the dependency docopt here

We could move the hook scripts here but I prefer not to. It does not matter actually where they live for the end user. Why should I require people to install docopt if they don't want to use the hook? 99% of people actually don't user the styler as a pre-commit hook, they use it in other ways (Addin, R API).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants