[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin marked 4 inline comments as done.
jrmolin added inline comments.


================
Comment at: clang/include/clang/Format/Format.h:827
+  /// \code
+  ///   someFunction(
+  ///       int argument1,
----------------
HazardyKnusperkeks wrote:
> That's not a valid declaration (missing return type), so not a good example.
fixed. creating a new patch with all the requested context.


================
Comment at: clang/include/clang/Format/Format.h:831
+  /// \endcode
+  bool AlwaysBreakBeforeFunctionParameters;
+
----------------
HazardyKnusperkeks wrote:
> HazardyKnusperkeks wrote:
> > Please sort alphabetically. (So above `AlwaysBreakBeforeMultilineStrings`.)
> You are missing the \since 17.
`\version 17`? I added this locally and am generating a new patch, with all the requested context.


================
Comment at: clang/include/clang/Format/Format.h:831
+  /// \endcode
+  bool AlwaysBreakBeforeFunctionParameters;
+
----------------
jrmolin wrote:
> HazardyKnusperkeks wrote:
> > HazardyKnusperkeks wrote:
> > > Please sort alphabetically. (So above `AlwaysBreakBeforeMultilineStrings`.)
> > You are missing the \since 17.
> `\version 17`? I added this locally and am generating a new patch, with all the requested context.
sorted. creating a patch with requested context


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:4740-4742
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+      !Right.is(tok::r_paren)) {
+    if (Left.Previous) {
----------------
HazardyKnusperkeks wrote:
> Merge the `if`s.
I'm not sure I collapsed what you wanted. I am generating a new patch with all the context.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125171/new/

https://reviews.llvm.org/D125171