-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
[clang-format] Fix breaking change in 20.1.0 by automatically falling back to Cpp formatting if C formatting fails #132832
Comments
@llvm/issue-subscribers-clang-format Author: Sean McManus (sean-mcmanus)
@owenca The PR at https://github.com//pull/128287 broke the case where a .c file is formatted in a folder with a .clang-format with Language: Cpp.
Can you make it not be a breaking change by automatically falling back to formatting with Cpp if no C language block is specified in .clang-format instead of giving an error? Or do you recommend a higher level tooling layer detect the failure and change use --assume-filename=file.cpp instead of file.c as a workaround or what? |
Additional context: We are maintainers of the Microsoft C/C++ extension for VS Code and this issue was brought to our attention by one of our users. |
Can you provide a reproducer? It seems to work for me.
|
Using the config file in microsoft/vscode-cpptools#13406 also works for me. |
@owenca the code in the cpptools#13406 is a reproducible on my side. With the latest release, if you pass the This breaks practically everything as now clang-format has to be updated and eventually vscode cpp tools does this call:
When you run it with
You get:
But when you run it with
Then no error and it produces the expected result. Ultimately, I think what we shall have is:
|
I still couldn't reproduce it:
|
@owenca I may know why you're not reprong it -- adding a .clang-format with BasedOnStyle: LLVM in the parent folder causes it to not repro for me -- can you check your parent folders for any .clang-format files? When I remove/that file the bug start to repro. I just have a .clang-format in my current working directory where the source file is. My repro is on Windows, but I can check mac/Linux later today. |
@sean-mcmanus you are right! |
@MaJerle if you delete the |
Yup, I confirm this is the case. But this will still impact other users in general. Because C and Cpp will have to be updated in their format file. |
@sean-mcmanus One workaround you may want to consider is to change the command to:
|
@owenca What is changed by that? Removing the --assume-filename? That flag also enables the include sorting format feature to properly sort the file's header include at the top. Also, we already pass the file text to clang-format via stdin, so I'm not sure how that would be different from using An example C language repo with a Cpp language in .clang-format is at https://github.com/leo-arch/clifm/blob/91e1e7f03adcc757882a310a4bac55220eb9e7d1/src/.clang-format#L4 . The comment next to it is |
If you need to use
|
Thank you for the quick response @owenca! |
Fix llvm#132832 (cherry picked from commit 05fb840)
@owenca The PR at #128287 broke the case where a .c file is formatted in a folder with a .clang-format with Language: Cpp (and no "Language: C").
Can you make it not be a breaking change by automatically falling back to formatting with Cpp if no C language block is specified in .clang-format instead of giving an error?
Or do you recommend a higher level tooling layer detect the failure and change use --assume-filename=file.cpp instead of file.c as a workaround or what?
i.e. command line
clang-format test.c
gives "Configuration file(s) do(es) not support C".The text was updated successfully, but these errors were encountered: