[PATCH] D32480: clang-format: Add CompactNamespaces option

Francois Ferrand via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 18 04:53:59 PDT 2017


Typz marked 2 inline comments as done.
Typz added inline comments.


================
Comment at: include/clang/Format/Format.h:153
+  /// \endcode
+  bool AllowSemicolonAfterNamespace;
+
----------------
Typz wrote:
> djasper wrote:
> > While I am not entirely opposed to this feature, I think it should be a separate patch.
> I totally agree, which is why I created 2 commits; however, since they modify the same code (and I am new to Phabricator) I could not find a way to upload them separately.
> 
> Is there a way? Or should I upload one, and upload the next one once the first one has been accepted?
OK, found the answer here: https://medium.com/@kurtisnusbaum/stacked-diffs-keeping-phabricator-diffs-small-d9964f4dcfa6


================
Comment at: include/clang/Format/Format.h:358
+  /// \endcode
+  bool BinPackNamespaces;
+
----------------
djasper wrote:
> This is not bin packing at all. Maybe CompactNamespaces? Or SingleLineNamespaces?
> 
> Also, could you clarify what happens if the namespaces don't fit within the column limit (both in the comment describing the flag and by adding tests for it)?
This is not binpacking, but has a similar effect and made the option name somewhat consistent with the other binpack options :-)
I'll change to CompactNamespaces then.


https://reviews.llvm.org/D32480





More information about the cfe-commits mailing list