[PATCH] D50535: Fix selective formatting of ObjC scope

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added a comment.

Nice! That looks very promising.

It still fails when we pass a longer range (maybe mimicking `PreviousRBrace` 
will help), e.g.:
Base code:

  @protocol A
   @optional
  // comment
  - (void)f;
  @end
  MACRO

formatted with `clang-format -lines=3:6 file.m` gives:

  @protocol A
   @optional
   // comment
   - (void)f;
   @end
   MACRO

This is not happening for Cpp, e.g.:
Base code:

  class A {
   void f ();
  // comment
  void g ();
  };
  MACRO

running `clang-format -lines=3:6 file.cpp` gives:

  class A {
   void f ();
   // comment
   void g ();
  };
  MACRO

I think it would be good to add all these tests to unit tests, but I see that 
an interface to pass line ranges is not very pleasant.


Repository:
  rC Clang

https://reviews.llvm.org/D50535



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50535: Fix selective formatting of ObjC scope

[Arnaud Coomans via Phabricator via cfe-commits]

acoomans added a comment.

@jolesiak let me know what you think


Repository:
  rC Clang

https://reviews.llvm.org/D50535



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50535: Fix selective formatting of ObjC scope

[Arnaud Coomans via Phabricator via cfe-commits]

acoomans added a comment.

I probably don't need a unit test AND a lit test, but I'm unsure which one is 
best here. Any suggestion?




Comment at: lib/Format/UnwrappedLineFormatter.cpp:1045-1047
+if (TheLine.First->Next && 
TheLine.First->Next->Tok.isObjCAtKeyword(tok::objc_end)) {
+  ContinueFormatting = false;
+}

Note to self: change the style to match `PreviousRBrace`


Repository:
  rC Clang

https://reviews.llvm.org/D50535



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50535: Fix selective formatting of ObjC scope

[Arnaud Coomans via Phabricator via cfe-commits]

acoomans created this revision.
acoomans added a reviewer: cfe-commits.

ObjC scopes gets formatted beyond the `@end` directive, when formatting 
specific lines (`clang-format -lines=x:x`).

This changeset fixes formatting of `@end` the same way that `r_braces` are.


Repository:
  rC Clang

https://reviews.llvm.org/D50535

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  test/Format/adjust-indent-objc.m
  unittests/Format/FormatTestSelective.cpp


Index: unittests/Format/FormatTestSelective.cpp
===
--- unittests/Format/FormatTestSelective.cpp
+++ unittests/Format/FormatTestSelective.cpp
@@ -584,6 +584,20 @@
  15, 0));
 }
 
+TEST_F(FormatTestSelective, StopFormattingWhenLeavingObjCScope) {
+EXPECT_EQ("@protocol A\n"
+  " - (void)f;\n"
+  " - (void)g;\n"
+  "@end\n"
+  "MACRO",
+  format("@protocol A\n"
+ " - (void)f;\n"
+ "- (void)g;\n"  // Format here.
+ "@end\n"
+ "MACRO",
+ 25, 0));
+}
+
 TEST_F(FormatTestSelective, SelectivelyRequoteJavaScript) {
   Style = getGoogleStyle(FormatStyle::LK_JavaScript);
   EXPECT_EQ(
Index: test/Format/adjust-indent-objc.m
===
--- /dev/null
+++ test/Format/adjust-indent-objc.m
@@ -0,0 +1,13 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -lines=4:4 \
+// RUN:   | FileCheck -strict-whitespace %s
+
+@protocol A
+// CHECK: @protocol A
+ @optional
+// CHECK: {{^ @optional}}
+- (void)f;
+// CHECK: {{^ - \(void\)f;}}
+@end
+// CHECK: {{^@end}}
+MACRO
+// CHECK: {{^MACRO}}
\ No newline at end of file
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -1042,6 +1042,10 @@
 (TheLine.Level == RangeMinLevel && !PreviousRBrace &&
  !TheLine.startsWith(tok::r_brace));
 
+if (TheLine.First->Next && 
TheLine.First->Next->Tok.isObjCAtKeyword(tok::objc_end)) {
+  ContinueFormatting = false;
+}
+
 bool FixIndentation = (FixBadIndentation || ContinueFormatting) &&
   Indent != TheLine.First->OriginalColumn;
 bool ShouldFormat = TheLine.Affected || FixIndentation;


Index: unittests/Format/FormatTestSelective.cpp
===
--- unittests/Format/FormatTestSelective.cpp
+++ unittests/Format/FormatTestSelective.cpp
@@ -584,6 +584,20 @@
  15, 0));
 }
 
+TEST_F(FormatTestSelective, StopFormattingWhenLeavingObjCScope) {
+EXPECT_EQ("@protocol A\n"
+  " - (void)f;\n"
+  " - (void)g;\n"
+  "@end\n"
+  "MACRO",
+  format("@protocol A\n"
+ " - (void)f;\n"
+ "- (void)g;\n"  // Format here.
+ "@end\n"
+ "MACRO",
+ 25, 0));
+}
+
 TEST_F(FormatTestSelective, SelectivelyRequoteJavaScript) {
   Style = getGoogleStyle(FormatStyle::LK_JavaScript);
   EXPECT_EQ(
Index: test/Format/adjust-indent-objc.m
===
--- /dev/null
+++ test/Format/adjust-indent-objc.m
@@ -0,0 +1,13 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s | clang-format -lines=4:4 \
+// RUN:   | FileCheck -strict-whitespace %s
+
+@protocol A
+// CHECK: @protocol A
+ @optional
+// CHECK: {{^ @optional}}
+- (void)f;
+// CHECK: {{^ - \(void\)f;}}
+@end
+// CHECK: {{^@end}}
+MACRO
+// CHECK: {{^MACRO}}
\ No newline at end of file
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -1042,6 +1042,10 @@
 (TheLine.Level == RangeMinLevel && !PreviousRBrace &&
  !TheLine.startsWith(tok::r_brace));
 
+if (TheLine.First->Next && TheLine.First->Next->Tok.isObjCAtKeyword(tok::objc_end)) {
+  ContinueFormatting = false;
+}
+
 bool FixIndentation = (FixBadIndentation || ContinueFormatting) &&
   Indent != TheLine.First->OriginalColumn;
 bool ShouldFormat = TheLine.Affected || FixIndentation;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits