Page MenuHomePhabricator

[clang-format] Preserve include blocks in ObjC Google style
ClosedPublic

Authored by krasimir on Apr 4 2019, 6:15 AM.

Details

Summary

r357567 started to regroup include block for Google style; it was meant to apply
only for C++. This patch reverts this for ObjC.

Diff Detail

Repository
rL LLVM

Event Timeline

krasimir created this revision.Apr 4 2019, 6:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2019, 6:15 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
ioeric added inline comments.Apr 4 2019, 6:20 AM
lib/Format/Format.cpp
787 ↗(On Diff #193696)

maybe we should also only use regroup for cpp? regroup is only supported in sortCppIncludes after all.

krasimir marked 2 inline comments as done.Apr 4 2019, 6:33 AM
krasimir added inline comments.
lib/Format/Format.cpp
787 ↗(On Diff #193696)

Yeah, but they are just not used for unrelated languages. This argument applies to the other fields in the IncludeStyle group above this. I'd lean on keeping this as-is.

ioeric accepted this revision.Apr 4 2019, 6:34 AM
This revision is now accepted and ready to land.Apr 4 2019, 6:34 AM

Why would we want this to be different in Obj-C and C++?

krasimir marked an inline comment as done.Apr 4 2019, 6:46 AM

Why would we want this to be different in Obj-C and C++?

Only the C++ Style Guide had been updated.
Practically, clang-format would need to be updated to recognize ObjC-specific code patterns better before being able to apply regrouping to ObjC code in the wild.

I'd expect that sometime in the future the ObjC-style catches up.

krasimir updated this revision to Diff 193699.Apr 4 2019, 7:01 AM
  • Add a note about intent
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2019, 7:02 AM

Please don't land changes with open discussion.

Objective-C++ basically is C++ code. I don't think it makes sense to make a distinction here.

Looks good to me.

cfe/trunk/lib/Format/Format.cpp
881

nit: "headers"