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

WIP: fix pre-commit-clang-format not working on Windows #390

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

Coder-Manan
Copy link

Description

The pre commit hook which checks for clang version installed doesn't work on Windows OS as expected. This PR aims at fixing that.

Fixes #370

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests/screenshots (if any) that prove my fix is effective or that my feature works.
  • I have tested the tests implicated (if any) by my own code and they pass (make test or ctest -VV -R <test-name>).
  • If my change is significant or breaking, I have passed all tests with ./docker-compose.sh test &> output and attached the output.
  • I have tested my code with OPTION_BUILD_SANITIZER or ./docker-compose.sh test-sanitizer &> output and OPTION_TEST_MEMORYCHECK.
  • I have tested my code with OPTION_BUILD_THREAD_SANITIZER or ./docker-compose.sh test-thread-sanitizer &> output.
  • I have tested with Helgrind in case my code works with threading.
  • I have run make clang-format in order to format my code and my code follows the style guidelines.

If you are unclear about any of the above checks, have a look at our documentation here.

@Coder-Manan
Copy link
Author

This is a WIP PR made to track the work done for this issue. Please don't merge this yet

@Coder-Manan
Copy link
Author

I will resume work after 4 march, since I have my college exams till then

@viferga viferga marked this pull request as draft March 1, 2023 16:06
@viferga
Copy link
Member

viferga commented Mar 1, 2023

Please set the PR as draft when it is WIP.

@pkspyder007
Copy link
Contributor

@Coder-Manan any updates on this one?

@Coder-Manan
Copy link
Author

I am currently learning about both the scripts, will work after that.

@viferga
Copy link
Member

viferga commented Mar 27, 2023

It would be interesting to have it in a single script: https://codegolf.stackexchange.com/a/66700

@Coder-Manan
Copy link
Author

I have started porting the script to powershell, piece by piece. I will be highly obliged if you review it and tell me any corrections which may be needed

@Coder-Manan
Copy link
Author

Coder-Manan commented Apr 2, 2023

While porting the script I found that we may need to port githooks\canonicalize_filename.sh as well(because it is being run in the clang-format githook). I'll do that in a separate PR if needed

@viferga
Copy link
Member

viferga commented Sep 13, 2023

I have created a new subfolder for implementing this on windows only. Now it is cross-platform, but it lacks windows implementation, it won't give errors with windows anymore: 69fad4b

@Coder-Manan If you are still interested you can continue implementing it in this folder: https://github.com/metacall/core/tree/develop/githooks/win32

Feel free to implement everything in the same PR.

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.

pre-commit-clang-format not working on windows
3 participants