Page MenuHomePhabricator

[clang-format] Also look for .{ext}.clang-format file
AbandonedPublic

Authored by wanders on Oct 7 2019, 4:10 AM.

Details

Reviewers
MyDeveloperDay
klimek
owenpan
Group Reviewers
Restricted Project
Summary

E.g: When formatting foo.cpp it will look for .cpp.clang-format

This makes it easy to different formatting for .c/.h files and
.cpp/.hpp files within the same directories.

Diff Detail

Event Timeline

wanders created this revision.Oct 7 2019, 4:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2019, 4:10 AM

Thanks for the patch, I think I understand what you are trying to do, but I have a few questions.

Is the premise here that you need slightly different styles for .cpp than for .h? could you explain why?

whilst I can see there is value in having such fine control, could you explain a little what leads you to needing that? because it feels like you are almost saying a bug in clang-format prevents me from using a single file and I'd like to understand if that is the case?

clang/lib/Format/Format.cpp
2606

if Extension is a std::string use if (!Extension.empty())

I'm not seeing any merrit this brings over existing https://clang.llvm.org/docs/ClangFormatStyleOptions.html Language option.

The "Language" option can not distinguish between C and C++.

We have projects which contains both C and C++ code. Using different style (yes..) for C and C++. Our C++ headers are named hpp.

In our toplevel we have the following symlinks
.c.clang-format -> clang-format/c.clang-format
.h.clang-format -> clang-format/c.clang-format
.cpp.clang-format -> clang-format/cpp.clang-format
.hpp.clang-format -> clang-format/cpp.clang-format

Before this we had a horrible wrapper script that used cd into different directories depending on extension, and those directories had different .clang-format files, and then had to run clang-format from stdin.

An alternative implementation I did consider was to add some kind of path matching in the configuration file doing multiple yaml documents in the configuration file. So in my case it would be something like:

---
Language: Cpp
IndentWidth: 4
TabWidth: 4
MoreStuff...
---
PathMatch: [ *.cpp, *.hpp ]
SpaceBeforeParen: ControlStatements
---
PathMatch [ *.c, *.h ]
SpaceBeforeParen: Never
---
clang/lib/Format/Format.cpp
2603

Nit: almost every file has an extension should this be 3?

The "Language" option can not distinguish between C and C++.

We have projects which contains both C and C++ code. Using different style (yes..) for C and C++. Our C++ headers are named hpp.

That seems like a much smaller bug/feature to fix ?

MyDeveloperDay requested changes to this revision.May 5 2020, 3:09 AM

Can we abandon this review now we have the style=file:<filename>?

This revision now requires changes to proceed.May 5 2020, 3:09 AM
wanders abandoned this revision.May 8 2020, 1:43 AM

Can we abandon this review now we have the style=file:<filename>?

Yes, lets abandon this.

clang/lib/Format/Format.cpp
2606

It is a StringRef (StringRef llvm::sys::path::extension(StringRef path)) but that also has empty(), thanks.