clang-format: Replaced PointerBindsToType with PointerBinding to allow third kind of formatting

roman at himmes.com roman at himmes.com
Wed Oct 9 09:14:47 PDT 2013


Quoting Daniel Jasper <djasper at google.com>:

> On Tue, Oct 8, 2013 at 1:19 PM, Roman Himmes <roman at himmes.com> wrote:
>
>> Hi,
>>
>> at work we have a project with a different style where the * or & is
>> placed between the type and the variable. An example is: "const std::string
>> & str". To make clang-format to recognize that kind of formatting I created
>> a small patch, see below.
>>
>> I removed the old boolean setting "PointerBindsToType" and replaced it
>> with a "PointerBinding" option with the three settings: Left, Middle and
>> Right.
>>
>> Modified:
>>     include/clang/Format/Format.h
>>     lib/Format/Format.cpp
>>     unittests/Format/FormatTest.cpp
>>     lib/Format/TokenAnnotator.cpp
>>
>> What do you think of this patch?
>>
>
> - This would break everybody who already has a .clang-format file
> somewhere. We really need to make changes to the configuration options in a
> backwards compatible fashion if at all possible. We probably can change the
> type of the configuration variable, but we need to adapt the config file
> reader to still accept PointerBindsToType and set PointerBinding
> appropriately.

Thank you very much for your feedback. I am going to make it backwards  
compatible.


> - This would need a lot of additional tests to verify that we actually
> format the different cases correctly (duplicate at least a few tests from
> UnderstandsUsesOfStarAndAmp).

ok, I am going to add unit tests as well.

-- snip

>>  template <>
>>
>> +struct  
>> ScalarEnumerationTraits<clang::format::FormatStyle::PointerBindingKind>  
>> {
>>
>> +  static void enumeration(IO &IO,
>>
>> +                           
>> clang::format::FormatStyle::PointerBindingKind &Value) {
>>
>> +    IO.enumCase(Value, "Left",   clang::format::FormatStyle::PB_Left);
>>
>> +    IO.enumCase(Value, "Middle", clang::format::FormatStyle::PB_Middle);
>>
>> +    IO.enumCase(Value, "Right",  clang::format::FormatStyle::PB_Right);
>>
>>
> While this alignment might seem nice, I don't think it adds a lot of value.
> I'd rather keep this file clang-format clean.

At first I thought that this alignment is quite exotic, but entering  
"const std::sting & str" into goole I found that there are a few  
libraries out there using this alignment.
Poco for example. So I see only 2 choices to support this style:
Either have a config option like in this patch, or removing that  
explicit option and have that Middle style only auto detected.


-- snip
>>
>>  @@ -980,9 +990,9 @@
>>
>>      }
>>
>>      if (Style.DerivePointerBinding) {
>>
>>        if (CountBoundToType > CountBoundToVariable)
>>
>> -        Style.PointerBindsToType = true;
>>
>> +        Style.PointerBinding = FormatStyle::PB_Left;
>>
>>        else if (CountBoundToType < CountBoundToVariable)
>>
>> -        Style.PointerBindsToType = false;
>>
>> +        Style.PointerBinding = FormatStyle::PB_Right;
>>
>>
> Wouldn't we want to detect all three kinds of bindings.
>

Ok, right. I am going to implement this.


Best regards,

Roman Himmes






More information about the cfe-commits mailing list