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

specify which tool(s) to install #27

Closed
2bndy5 opened this issue Aug 15, 2022 · 5 comments · Fixed by #60
Closed

specify which tool(s) to install #27

2bndy5 opened this issue Aug 15, 2022 · 5 comments · Fixed by #60
Labels
enhancement New feature or request

Comments

@2bndy5
Copy link
Contributor

2bndy5 commented Aug 15, 2022

It would be nice to let the user choose which tool(s) to install. This way we could only install clang-tidy in the cpp-linter-action (composite form) if inputs.tidy-checks != '-*. Or we could avoid installing clang-format if the inputs.style != ''.

Since the releases of binary builds also include the clang-query tool, users could specify that as well.

Implementation

Option 1

clang-tools -t clang-format -t clang-tidy -t clang-query

Option 2

clang-tools -t clang-format,clang-tidy,clang-query

I haven't decided which way to support. Maybe there's a way to support either or both options.

I would expect the current default ( clang-tidy + clang-format) to be overridden when -t is specified.

@2bndy5 2bndy5 added the enhancement New feature or request label Aug 15, 2022
@shenxianpeng
Copy link
Collaborator

shenxianpeng commented Aug 17, 2022

It would be nice to support users in installing specific tools.

For cpp-linter-action, I think it's OK to provide both tools (including clang-format and clang-tidy) if the user only uses one of them. It will add a small amount of download time, but the logic is more simple than adding more if else to check users' inputs, less code fewer bugs 😆

@shenxianpeng
Copy link
Collaborator

clang-format or clang-tidy is installed with clang-tools command before running cpp-linter, so it should be hard to know which needs to be installed in the preparation stage

@2bndy5
Copy link
Contributor Author

2bndy5 commented Aug 18, 2022

This way we could only install clang-tidy in the cpp-linter-action (composite form) if inputs.tidy-checks != '-*. Or we could avoid installing clang-format if the inputs.style != ''.

This an advanced example. The defaults should suffice for our needs. What if someone wants to use this pkg to get the statically linked clang-query binary?

@shenxianpeng
Copy link
Collaborator

What if someone wants to use this pkg to get the statically linked clang-query binary?

Your implementation proposal like great, users could use this way to install clang-query binary.
Option 1
clang-tools -t clang-format -t clang-tidy -t clang-query
Option 2
clang-tools -t clang-format,clang-tidy,clang-query

@shenxianpeng
Copy link
Collaborator

shenxianpeng commented Jan 13, 2024

I saw that the user from the upstream repo of cpp-linter/clang-tools-static-binaries was asking to have clang-apply-replacements binary, so I added it to the latest release master-a2d1eff4. so this enhancement becomes more and more worth to implement.

I don't have much of an impression of how other tools implement similar functionality, and so far I prefer option 1 (option 2 is also not bad)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants