kicad-developers team mailing list archive

[Jeff Young <jeff@xxxxxxxxx>]

We do use (and encourage) single-line cases when it’s more readable that way.  This particularly comes up when mapping or converting something, such as:
switch( aFaceId )
{
case FACE_SCH:              name = KIFACE_PREFIX "eeschema";            break;
case FACE_PCB:              name = KIFACE_PREFIX "pcbnew";              break;
case FACE_CVPCB:            name = KIFACE_PREFIX "cvpcb";               break;
case FACE_GERBVIEW:         name = KIFACE_PREFIX "gerbview";            break;
case FACE_PL_EDITOR:        name = KIFACE_PREFIX "pl_editor";           break;
case FACE_PCB_CALCULATOR:   name = KIFACE_PREFIX "pcb_calculator";      break;
case FACE_BMP2CMP:          name = KIFACE_PREFIX "bitmap2component";    break;

default:
    wxASSERT_MSG( 0, wxT( "caller has a bug, passed a bad aFaceId" ) );
    return wxEmptyString;
}

or:

switch( aType )
{
case SCH_LABEL_T:        newtext = new SCH_LABEL( position, txt );        break;
case SCH_GLOBAL_LABEL_T: newtext = new SCH_GLOBALLABEL( position, txt );  break;
case SCH_HIER_LABEL_T:   newtext = new SCH_HIERLABEL( position, txt );    break;
case SCH_TEXT_T:         newtext = new SCH_TEXT( position, txt );         break;

default:
    wxASSERT_MSG( false, wxString::Format( "Invalid text type: %d.", aType ) );
    return;
}

or:

if( aRotateCCW )
{
    switch( m_orientation )
    {
    case PIN_RIGHT: m_orientation = PIN_UP;    break;
    case PIN_UP:    m_orientation = PIN_LEFT;  break;
    case PIN_LEFT:  m_orientation = PIN_DOWN;  break;
    case PIN_DOWN:  m_orientation = PIN_RIGHT; break;
    }
}
else
{
    switch( m_orientation )
    {
    case PIN_RIGHT: m_orientation = PIN_DOWN;  break;
    case PIN_UP:    m_orientation = PIN_RIGHT; break;
    case PIN_LEFT:  m_orientation = PIN_UP;    break;
    case PIN_DOWN:  m_orientation = PIN_LEFT;  break;
    }
}

Cheers,
Jeff.


> On 6 May 2019, at 21:09, John Beard <john.j.beard@xxxxxxxxx> 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).
> 
> 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