[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Micah S. via Phabricator via cfe-commits]

micah-s added a comment.

In D28462#1412556 , @MyDeveloperDay 
wrote:

> For those wishing to test the effectiveness of unlanded revisions like this 
> and to reduce the amount of time between Windows snapshot builds 
> (https://llvm.org/builds/), I have forked llvm-project and using AppVeyor  to 
> build a new x64 Windows clang-format-experimental.exe binary, on a 
> semi-automatic basis. (master branch clang-format with selected unlanded 
> revisions)
>
> https://github.com/mydeveloperday/clang-experimental/releases


Thanks @MyDeveloperDay! That is very helpful, and significantly reduced the 
overhead for testing.

I removed the `// clang-format off`/`on` markers around all of our aligned 
macros, applied the `AlignConsecurityMacros: true` setting to our 
.clang-format, and reformatted the affected files using the 
clang-format-experimental.exe binary.  My own experience was flawless.  The 
only issue we'll have to work around is that we have some longer macro sets 
that include unused values in a series.

For example:

  ...
  #define CL_SCAN_ALGORITHMIC   0x200
  //#define UNUSED  0x400
  #define CL_SCAN_PHISHING_BLOCKSSL 0x800
  ...

becomes

  ...
  #define CL_SCAN_ALGORITHMIC 0x200
  //#define UNUSED  0x400
  #define CL_SCAN_PHISHING_BLOCKSSL 0x800
  ...

I can't really complain though, because it's working as intended -- we'll just 
have to find a different way to indicate reserved/unused values in a series.  
I'll probably just do something like `CL_SCAN_UNUSED_0`/`1`/`2`/etc.
Big thumbs up here from an end user experience.


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

https://reviews.llvm.org/D28462



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Micah S. via Phabricator via cfe-commits]

micah-s added a comment.

In D28462#1413332 , @MyDeveloperDay 
wrote:

> Would you please consider logging your struct example from your blog at 
> https://bugs.llvm.org,  I see a large number of people using clang format 
> on/off in github for this and similar issues (including google code!), looks 
> like clang-format needs some struct breaking rules


Will do!


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

https://reviews.llvm.org/D28462



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Micah S. via Phabricator via cfe-commits]

micah-s added a comment.

@djasper @klimek @krasimir @sammccall @enyquist  Can I trouble you for an 
update?

In D28462#1492600 , @jkorous wrote:

> Hi @VelocityRa, just FYI - it's considered fine to ping your reviewers once 
> per week here if you've addressed their comments and there's no activity in 
> the review. Sometimes people just get distracted by other things.





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

https://reviews.llvm.org/D28462



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Micah S. via Phabricator via cfe-commits]

micah-s added a comment.

ClamAV recently started using clang-format.  We published this blog post about 
how we're using it: 
https://blog.clamav.net/2019/02/clamav-adopts-clang-format.html  One of the 
things I called out in the blog post is that we really want this feature, and 
presently we have to use the "// clang-format on/off" markers throughout our 
code mostly to keep our consecutive macros aligned.

I haven't found time to build clang-format with this patch to test it on our 
code base with the markers disabled.  If a review of my experience doing so 
will help get traction for this review in any way, let me know and I'll make 
the effort.


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

https://reviews.llvm.org/D28462



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