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