Page MenuHomePhabricator

[clang-format] [PR51412] AlignConsecutiveMacros fights with Visual Studio and resource.h
AcceptedPublic

Authored by MyDeveloperDay on Oct 18 2021, 10:43 AM.

Details

Summary

https://bugs.llvm.org/show_bug.cgi?id=51412

clang-format AlignConsecutiveMacros feature causes real problems when using Win32 resource.h files via the resource editor in Visual Studio when editing RC files.

VS produces the following for the resource.h files

...
// Microsoft Visual C++ generated include file.
// Used by MyTest.rc
//
#define IDP_OLE_INIT_FAILED             100
#define IDP_FAILED_TO_CREATE            102
#define ID_STATUSBAR_SHOW               108
#define ID_STATUSBAR_TEXT               109

Visual Studio generates a resource.h with Alignment macros which start at 40 characters, but AlignConsecutiveMacros will determine the starting point but assume a minimum of 0 meaning a clang-format will result in:

#define IDP_OLE_INIT_FAILED  100
#define IDP_FAILED_TO_CREATE 102
#define ID_STATUSBAR_SHOW    108
#define ID_STATUSBAR_TEXT    109

This is would be good until you make a new rc file change which results in VS writing out the resource.h (back in its own form) - (even clang-format VS plugin to format on save doesn't resolve this. The writing of resource.h seems outside of the normal save system)

This situation is made worse, if it encounters a long symbol, in the VS case it treats this as a one off, but clang-format will assume this is the new minimum.

#define IDP_OLE_INIT_FAILED             100
#define IDP_FAILED_TO_CREATE            102
#define ID_VERYVERYVERYVERY_LONG_LONG_LONG_LONG_RESOURCE 33221
#define ID_STATUSBAR_SHOW               108
#define ID_STATUSBAR_TEXT               109

and will become via clang-format

#define IDP_OLE_INIT_FAILED                              100
#define IDP_FAILED_TO_CREATE                             102
#define ID_VERYVERYVERYVERY_LONG_LONG_LONG_LONG_RESOURCE 33221
#define ID_STATUSBAR_SHOW                                108
#define ID_STATUSBAR_TEXT                                109

This patch contains 2 new options

  1. AlignConsecutiveMacrosMinWidth Sets the minimum to a fixed integer (which for VS should be 40), this means all id numbers will begin at 40
  2. AlignConsecutiveMacrosIgnoreMax is an option to allow very long macro names to break the Alignment, meaning the next line will reset back to the previous configured minimum (40 in our case)

These 2 option have functionality that could be useful on their own, however the goal is to allow AlignConsecutiveMacros to be useful for Windows Win32 developers and minimize the clang-format changes that the RC editor causes.

Diff Detail

Event Timeline

MyDeveloperDay requested review of this revision.Oct 18 2021, 10:43 AM
MyDeveloperDay created this revision.
MyDeveloperDay edited the summary of this revision. (Show Details)

Otherwise it looks good.

clang/include/clang/Format/Format.h
245

I before M, so please reorder. :)

This revision is now accepted and ready to land.Oct 19 2021, 1:27 PM