kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

On 08/26/2010 04:50 AM, Marco Mattila wrote:
> === modified file 'pcbnew/edit.cpp'
> --- pcbnew/edit.cpp	2010-08-17 10:41:44 +0000
> +++ pcbnew/edit.cpp	2010-08-26 09:46:16 +0000
> @@ -38,6 +38,7 @@
>      int         itmp;
>      INSTALL_DC( dc, DrawPanel );
>      BOARD_ITEM* DrawStruct = GetCurItem();
> +    MODULE* module;
>  
>      DrawPanel->CursorOff( &dc );
>  
> @@ -608,11 +609,23 @@
>              g_Drag_Pistes_On = false;
>              break;
>          }
> -        GetScreen()->m_Curseur = ((MODULE*) GetCurItem())->m_Pos;
> -        DrawPanel->MouseToCursorSchema();
> -        StartMove_Module( (MODULE*) GetCurItem(), &dc );
> +        module = (MODULE*) GetCurItem();
> +        if( module->IsLocked() )
> +        {
> +            wxString msg;
> +            msg.Printf( _( "Footprint %s found, but it is locked" ),
> +                        module->m_Reference->m_Text.GetData() );
> +            DisplayInfoMessage( this, msg );
> +            break;
>   

The above 'break' is fine, but makes the wrapping 'else' below not
necessary.  Instead that wrapped code could be promoted left one tab
stop and unwrapped from the else.  This is a common theme for several of
the edits in this file and I won't say the same thing about each one,
although it should be understood.  There is some advantage to keeping
things left, since it makes for longer lines of code.


> +        }
> +        else
> +        {
> +            GetScreen()->m_Curseur = ((MODULE*) GetCurItem())->m_Pos;
> +            DrawPanel->MouseToCursorSchema();
> +            StartMove_Module( (MODULE*) GetCurItem(), &dc );
> +        }
>          break;
> -
> +        
>   

The above blank line has 8 spaces after in it.  This is a bad editor
setting.


>      case ID_POPUP_PCB_GET_AND_MOVE_MODULE_REQUEST:      /* get module by name and move it */
>          SetCurItem( GetModuleByName() );
>          if( GetCurItem() )
> @@ -624,61 +637,106 @@
>  
>      case ID_POPUP_PCB_DELETE_MODULE:
>          DrawPanel->MouseToCursorSchema();
> -
> +        
>          // If the current Item is a pad, text module ...: Get its parent
>          if( GetCurItem()->Type() != TYPE_MODULE )
>              SetCurItem( GetCurItem()->GetParent() );
> -
> +        
>          if( !GetCurItem() || GetCurItem()->Type() != TYPE_MODULE )
>              break;
> -        if( Delete_Module( (MODULE*) GetCurItem(), &dc, true ) )
> -        {
> -            SetCurItem( NULL );
> +        module = (MODULE*) GetCurItem();
> +        if( module->IsLocked() )
> +        {
> +            wxString msg;
> +            msg.Printf( _( "Footprint %s found, but it is locked" ),
> +                        module->m_Reference->m_Text.GetData() );
> +            DisplayInfoMessage( this, msg );
> +            break;
> +        }
> +        else
> +        {
> +            if( Delete_Module( (MODULE*) GetCurItem(), &dc, true ) )
> +            {
> +                SetCurItem( NULL );
> +            }
>          }
>          break;
>  
>      case ID_POPUP_PCB_ROTATE_MODULE_COUNTERCLOCKWISE:
>          DrawPanel->MouseToCursorSchema();
> -
> +        
>          // If the current Item is a pad, text module ...: Get its parent
>          if( GetCurItem()->Type() != TYPE_MODULE )
>              SetCurItem( GetCurItem()->GetParent() );
> -
> +        
>          if( !GetCurItem() || GetCurItem()->Type() != TYPE_MODULE )
>              break;
> -
> -        if( !(GetCurItem()->m_Flags & IS_MOVED) ) /* This is a simple rotation, no other edition in progress */
> -            SaveCopyInUndoList(GetCurItem(), UR_ROTATED, ((MODULE*)GetCurItem())->m_Pos);
> -        Rotate_Module( &dc, (MODULE*) GetCurItem(), 900, true );
> +        module = (MODULE*) GetCurItem();
> +        if( module->IsLocked() )
> +        {
> +            wxString msg;
> +            msg.Printf( _( "Footprint %s found, but it is locked" ),
> +                        module->m_Reference->m_Text.GetData() );
> +            DisplayInfoMessage( this, msg );
> +            break;
> +        }
> +        else
> +        {
> +            if( !(GetCurItem()->m_Flags & IS_MOVED) ) /* This is a simple rotation, no other edition in progress */
> +                SaveCopyInUndoList(GetCurItem(), UR_ROTATED, ((MODULE*)GetCurItem())->m_Pos);
> +            Rotate_Module( &dc, (MODULE*) GetCurItem(), 900, true );
> +        }
>          break;
> -
> +        
>      case ID_POPUP_PCB_ROTATE_MODULE_CLOCKWISE:
>          DrawPanel->MouseToCursorSchema();
> -
> +        
>          // If the current Item is a pad, text module ...: Get its parent
>          if( GetCurItem()->Type() != TYPE_MODULE )
>              SetCurItem( GetCurItem()->GetParent() );
> -
> +        
>          if( !GetCurItem() || GetCurItem()->Type() != TYPE_MODULE )
>              break;
> -        if( !(GetCurItem()->m_Flags & IS_MOVED) ) /* This is a simple rotation, no other edition in progress */
> -            SaveCopyInUndoList(GetCurItem(), UR_ROTATED_CLOCKWISE, ((MODULE*)GetCurItem())->m_Pos);
> -        Rotate_Module( &dc, (MODULE*) GetCurItem(), -900, true );
> +        module = (MODULE*) GetCurItem();
> +        if( module->IsLocked() )
> +        {
> +            wxString msg;
> +            msg.Printf( _( "Footprint %s found, but it is locked" ),
> +                        module->m_Reference->m_Text.GetData() );
> +            DisplayInfoMessage( this, msg );
> +            break;
> +        }
> +        else
> +        {
> +            if( !(GetCurItem()->m_Flags & IS_MOVED) ) /* This is a simple rotation, no other edition in progress */
> +                SaveCopyInUndoList(GetCurItem(), UR_ROTATED_CLOCKWISE, ((MODULE*)GetCurItem())->m_Pos);
> +            Rotate_Module( &dc, (MODULE*) GetCurItem(), -900, true );
> +        }
>          break;
>  
>      case ID_POPUP_PCB_CHANGE_SIDE_MODULE:
>          DrawPanel->MouseToCursorSchema();
> -
> +        
>          // If the current Item is a pad, text module ...: Get its parent
>          if( GetCurItem()->Type() != TYPE_MODULE )
>              SetCurItem( GetCurItem()->GetParent() );
>          if( !GetCurItem() || GetCurItem()->Type() != TYPE_MODULE )
>              break;
> -
> -        if( !(GetCurItem()->m_Flags & IS_MOVED) ) /* This is a simple flip, no other edition in progress */
> -            SaveCopyInUndoList(GetCurItem(), UR_FLIPPED, ((MODULE*)GetCurItem())->m_Pos);
> -
> -        Change_Side_Module( (MODULE*) GetCurItem(), &dc );
> +        module = (MODULE*) GetCurItem();
> +        if( module->IsLocked() )
> +        {
> +            wxString msg;
> +            msg.Printf( _( "Footprint %s found, but it is locked" ),
> +                        module->m_Reference->m_Text.GetData() );
> +            DisplayInfoMessage( this, msg );
> +            break;
> +        }
> +        else
> +        {
> +            if( !(GetCurItem()->m_Flags & IS_MOVED) ) /* This is a simple flip, no other edition in progress */
> +                SaveCopyInUndoList(GetCurItem(), UR_FLIPPED, ((MODULE*)GetCurItem())->m_Pos);
> +            Change_Side_Module( (MODULE*) GetCurItem(), &dc );
> +        }
>          break;
>  
>      case ID_POPUP_PCB_EDIT_MODULE:
> @@ -693,22 +751,50 @@
>          break;
>  
>      case ID_POPUP_PCB_DRAG_PAD_REQUEST:
> -        g_Drag_Pistes_On = true;
> -        DrawPanel->MouseToCursorSchema();
> -        StartMovePad( (D_PAD*) GetCurItem(), &dc );
> +        module = (MODULE*) GetCurItem()->GetParent();
> +        if( !module || module->Type() != TYPE_MODULE )
> +            break;
> +        if( module->IsLocked() )
> +        {
> +            wxString msg;
> +            msg.Printf( _( "The parent (%s) of the pad is locked" ),
> +                        module->m_Reference->m_Text.GetData() );
> +            DisplayInfoMessage( this, msg );
> +            break;
> +        }
> +        else
> +        {
> +            g_Drag_Pistes_On = true;
> +            DrawPanel->MouseToCursorSchema();
> +            StartMovePad( (D_PAD*) GetCurItem(), &dc );
> +        }
>          break;
>  
>      case ID_POPUP_PCB_MOVE_PAD_REQUEST:
> -        g_Drag_Pistes_On = false;
> -        DrawPanel->MouseToCursorSchema();
> -        StartMovePad( (D_PAD*) GetCurItem(), &dc );
> +        module = (MODULE*) GetCurItem()->GetParent();
> +        if( !module || module->Type() != TYPE_MODULE )
> +            break;
> +        if( module->IsLocked() )
> +        {
> +            wxString msg;
> +            msg.Printf( _( "The parent (%s) of the pad is locked" ),
> +                        module->m_Reference->m_Text.GetData() );
> +            DisplayInfoMessage( this, msg );
> +            break;
> +        }
> +        else
> +        {
> +            g_Drag_Pistes_On = false;
> +            DrawPanel->MouseToCursorSchema();
> +            StartMovePad( (D_PAD*) GetCurItem(), &dc );
> +        }
>          break;
> -
> +        
>      case ID_POPUP_PCB_EDIT_PAD:
>          InstallPadOptionsFrame( (D_PAD*) GetCurItem() );
>          DrawPanel->MouseToCursorSchema();
>          break;
> -
> +        
>      case ID_POPUP_PCB_IMPORT_PAD_SETTINGS:
>          DrawPanel->MouseToCursorSchema();
>          SaveCopyInUndoList( GetCurItem()->GetParent(), UR_CHANGED );
>
> === modified file 'pcbnew/hotkeys.cpp'
> --- pcbnew/hotkeys.cpp	2010-08-17 17:45:45 +0000
> +++ pcbnew/hotkeys.cpp	2010-08-26 09:33:52 +0000
> @@ -738,24 +738,11 @@
>          break;
>  
>      case TYPE_MODULE:
>   


Please explain the reason for removing this code:


> -    {
> -        MODULE* module = (MODULE*) item;
> -
> -        // a footprint is found, but locked or on an other layer
> -        if( module->IsLocked() )
> -        {
> -            wxString msg;
> -            msg.Printf( _( "Footprint %s found, but locked" ),
> -                       module->m_Reference->m_Text.GetData() );
> -            DisplayInfoMessage( this, msg );
> -            break;
> -        }
>          if( aIdCommand == HK_MOVE_ITEM )
>              evt_type = ID_POPUP_PCB_MOVE_MODULE_REQUEST;
>          if( aIdCommand == HK_DRAG_ITEM )
>              evt_type = ID_POPUP_PCB_DRAG_MODULE_REQUEST;
> -    }
> -    break;
> +        break;
>  
>      case TYPE_PAD:
>          if( aIdCommand == HK_MOVE_ITEM )
> @@ -834,15 +821,6 @@
>      {
>      case TYPE_MODULE:
>      {
>   

and for removing this code.


> -         MODULE* module = (MODULE*) item;
> -        if( module->IsLocked() )
> -        {
> -            wxString msg;
> -            msg.Printf( _( "Footprint %s is locked" ),
> -                       module->m_Reference->m_Text.GetData() );
> -            DisplayInfoMessage( this, msg );
> -            break;
> -        }
>          if( aIdCommand == HK_ROTATE_ITEM )                      // Rotation
>              evt_type = ID_POPUP_PCB_ROTATE_MODULE_COUNTERCLOCKWISE;
>          if( aIdCommand == HK_FLIP_FOOTPRINT )                   // move to other side
>
>   

Dick