Page MenuHomePhabricator

[clang-format] Keep protobuf "package" statement on one line
ClosedPublic

Authored by dchai on Mar 20 2019, 7:41 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

dchai created this revision.Mar 20 2019, 7:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 20 2019, 7:41 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MyDeveloperDay added inline comments.
lib/Format/TokenAnnotator.cpp
1079 ↗(On Diff #191627)

Nit:

could we use

CurrentToken->isOneOf(Keywords.kw_option,Keyswords.kw_package)
dchai updated this revision to Diff 191734.Mar 21 2019, 10:22 AM

Use "isOneOf" instead of two calls to "is".

dchai marked an inline comment as done.Mar 21 2019, 10:23 AM
This revision is now accepted and ready to land.Mar 23 2019, 7:22 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2019, 7:44 AM
hokein added a subscriber: hokein.Mar 25 2019, 8:45 AM

This patch has caused a regression issue, see the test:

verifyFormat("// Detached comment\n\n"
                 "// Leading comment\n"
                 "syntax = \"proto2\"; // trailing comment\n\n"
                 "// in foo.bar package\n"
                 "package foo.bar; // foo.bar package\n");

I have reverted it in rL356912.

dchai added a comment.Mar 25 2019, 1:20 PM

Just to confirm, the regression is in the number of spaces before a trailing comment?

Before (2 spaces):

package foo.bar;  // foo.bar package

After (1 space):

package foo.bar; // foo.bar package

Top-level options are handled the same way. I'll see if I can address these both.