kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

John,

On 5/6/19 4:09 PM, John Beard wrote:
> On 06/05/2019 17:51, Reece R. Pollack wrote:
>> John,
>>
>> I've already jumped to clang-format 6.0, which is one of the optional
>> installs for Mint 18. That works, once you get all the symlinks fixed, 
> 
> Good to know, thanks for the update.
> 
>> except it keeps wanting to reformat my switch statements like this,
>> which is contrary to the KiCad coding standards:
>>
>> @@ -148,15 +130,9 @@ int PCB_ORIGIN_TRANSFORM_Y_ABS::FromDisplay( int
>> aValue ) const
>>       case ORIGIN_REFERENCE_PAGE:
>>           // No-op
>>           break;
>> -    case ORIGIN_REFERENCE_AUX:
>> -        origin = m_PcbBaseFrame->GetAuxOrigin().y;
>> -        break;
>> -    case ORIGIN_REFERENCE_GRID:
>> -        origin = m_PcbBaseFrame->GetGridOrigin().y;
>> -        break;
>> -    default:
>> -        wxASSERT(false);
>> -        break;
>> +    case ORIGIN_REFERENCE_AUX: origin =
>> m_PcbBaseFrame->GetAuxOrigin().y; break;
>> +    case ORIGIN_REFERENCE_GRID: origin =
>> m_PcbBaseFrame->GetGridOrigin().y; break;
>> +    default: wxASSERT( false ); break;
>>       }
>>
>>       // Invert the direction if needed
> 
> This is a weird style that I don't personally like, and IMO goes against
> the style implied by our "spacious" bracing and newline-before-if
> policies. But there are quite some uses of it. It's enforced by this line:
> 
>     AllowShortCaseLabelsOnASingleLine: true
> 
> It's the only line-condensing option we have set:
> 
>     AllowShortBlocksOnASingleLine: false
>     AllowShortCaseLabelsOnASingleLine: true # the only true here
>     AllowShortFunctionsOnASingleLine: false
>     AllowShortIfStatementsOnASingleLine: false
>     AllowShortLoopsOnASingleLine: false
> 
> If the formatter is proposing a change that goes against the existing
> style in the code area in question, I do not think it's controversial to
> ignore its suggestion.
> 
> @Wayne, what do you think: is this enforcement representative of the
> right style? Should we change AllowShortCaseLabelsOnASingleLine to
> false? (+1 from me).

I would rather not have clang-format force this change.  I am OK if the
developer prefers this for short case labels but if they prefer a
separate line that's fine.  I prefer everything on more than one line
but if I'm changing code that is already formatted on a single line, I
don't change it.  We didn't specify this in the coding policy so it's a
don't care but I would ask you to change it if you your case labels were
complex and you had them on a single line.

Wayne

> 
> Cheers,
> 
> John
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp