Sitelet https://github.com/pre-commit/pre-commit/pull/2293
Skip to content

coursier: Add support for both cs and coursier executable names - #2293

Merged
asottile merged 1 commit into
pre-commit:masterfrom
Holzhaus:coursier-executable-names
Mar 17, 2022
Merged

asottile merged 1 commit into
pre-commit:masterfrom
Holzhaus:coursier-executable-names

Conversation

@Holzhaus

Copy link
Copy Markdown
Contributor

On some systems, the executable might be named coursier instead of
cs. For example, this is the case on Arch Linux when using the AUR
package, or when following the official instructions when installing the
JAR-based launcher:

https://get-coursier.io/docs/cli-installation#jar-based-launcher

Comment thread pre_commit/languages/coursier.py Outdated
helpers.assert_no_additional_deps('coursier', additional_dependencies)

# Support both possible executable names (either "cs" or "coursier")
executable = next(filter(helpers.exe_exists, ('cs', 'coursier')), None)

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.

unnecessary FP -- we avoid map / filter / reduce

this should be exe('cs') or exe('coursier') once you find the right function (helpers.exe_exists isn't it -- that returns a bool)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this should be exe('cs') or exe('coursier') once you find the right function (helpers.exe_exists isn't it -- that returns a bool)

Do you want me to write a new one or is there something that I can use? shutil.which would be a candiate, or maybe pre_commit.parse_shebang.find_executable which kind of duplicates that functionality.

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.

find_executable is the right one probably yes

On some systems, the executable might be named `coursier` instead of
`cs`. For example, this is the case on Arch Linux when using the AUR
package, or when following the official instructions when installing the
JAR-based launcher:

  https://get-coursier.io/docs/cli-installation#jar-based-launcher
@Holzhaus
Holzhaus force-pushed the coursier-executable-names branch from 7846572 to 678ef6b Compare March 17, 2022 21:31

@asottile 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.

thanks!

@asottile
asottile merged commit 4421cb9 into pre-commit:master Mar 17, 2022
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.

2 participants