[PATCH] D48034: [clang-format] Discourage breaks in submessage entries

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a subscriber: cfe-commits.

Currently clang-format allows this for text protos:

  submessage:
      { key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' }

when it is under the column limit and when putting it all on one line exceeds 
the column limit.

This is not a very intuitive formatting, so I'd prefer having

  submessage: {
    key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
  }

instead, even if it takes one line more.

This is just a rough sketch of an approach to achieve this: add a penalty for 
breaking before the `{`.
Want to discuss the validity of this approach.


Repository:
  rC Clang

https://reviews.llvm.org/D48034

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestTextProto.cpp


Index: unittests/Format/FormatTestTextProto.cpp
===================================================================
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -670,5 +670,11 @@
                "}");
 }
 
+TEST_F(FormatTestTextProto, TODO) {
+  verifyFormat("submessage: {\n"
+               "  key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "}");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -22,6 +22,9 @@
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "format-indenter"
+#define DBG(A)                                                                 
\
+  llvm::dbgs() << __func__ << ":" << __LINE__ << ":" << #A << " = " << A       
\
+               << "\n";
 
 namespace clang {
 namespace format {
@@ -717,6 +720,7 @@
                                                  bool DryRun) {
   FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *State.NextToken->Previous;
+  DBG(Current.TokenText);
 
   // Extra penalty that needs to be added because of the way certain line
   // breaks are chosen.
@@ -743,6 +747,13 @@
        State.Stack.back().BreakBeforeParameter))
     Penalty += Style.PenaltyBreakFirstLessLess;
 
+  // Breaking before the "{" in a text proto submessage is not desirable.
+  if (Style.Language == FormatStyle::LK_TextProto) {
+    if (Current.is(tok::l_brace)) {
+      Penalty += Style.PenaltyExcessCharacter;
+    }
+  }
+
   State.Column = getNewLineColumn(State);
 
   // Indent nested blocks relative to this column, unless in a very specific



Index: unittests/Format/FormatTestTextProto.cpp
===================================================================
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -670,5 +670,11 @@
                "}");
 }
 
+TEST_F(FormatTestTextProto, TODO) {
+  verifyFormat("submessage: {\n"
+               "  key: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n"
+               "}");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -22,6 +22,9 @@
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "format-indenter"
+#define DBG(A)                                                                 \
+  llvm::dbgs() << __func__ << ":" << __LINE__ << ":" << #A << " = " << A       \
+               << "\n";
 
 namespace clang {
 namespace format {
@@ -717,6 +720,7 @@
                                                  bool DryRun) {
   FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *State.NextToken->Previous;
+  DBG(Current.TokenText);
 
   // Extra penalty that needs to be added because of the way certain line
   // breaks are chosen.
@@ -743,6 +747,13 @@
        State.Stack.back().BreakBeforeParameter))
     Penalty += Style.PenaltyBreakFirstLessLess;
 
+  // Breaking before the "{" in a text proto submessage is not desirable.
+  if (Style.Language == FormatStyle::LK_TextProto) {
+    if (Current.is(tok::l_brace)) {
+      Penalty += Style.PenaltyExcessCharacter;
+    }
+  }
+
   State.Column = getNewLineColumn(State);
 
   // Indent nested blocks relative to this column, unless in a very specific

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