kicad-developers team mailing list archive

[Maciej Sumiński <maciej.suminski@xxxxxxx>]

Hi Jeff,

I apologize for long delay. I have just reviewed and tested your patch
and the changes look fine, but there is one thing that needs to be
addressed before they can be accepted. Dragging a footprint with mouse
cursor does not work anymore, cursor simply gets stuck at the drag
origin position. Once it is fixed, I am willing to push your patch.

Please also consider that attached patch that fixes the code formatting.

Cheers,
Orson

On 01/09/2018 06:38 PM, Jeff Young wrote:
> The heat gets bumped up for multiple reports or when people click “this bug affects me too”.
> 
> Patch uploaded.
> 
> https://bugs.launchpad.net/kicad/+bug/1708869 <https://bugs.launchpad.net/kicad/+bug/1708869>
> 
> (The duplicate: https://bugs.launchpad.net/kicad/+bug/1503679 <https://bugs.launchpad.net/kicad/+bug/1503679> )
> 
> Cheers,
> Jeff.
> 
>> On 9 Jan 2018, at 16:06, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>
>> Hey Jeff,
>>
>> I'm not sure what a heat of 22 even means?  I don't see any comments or
>> suggestions in the bug report where lots of devs and/or users gave it a
>> big thumbs up.  I'm talking about getting some input on the concept and
>> testing on a patch from other devs and users.  I can't remember, did you
>> supply a patch for this?  I don't see one on the bug report.  I need to
>> review and test it at a minimum.
>>
>> Cheers,
>>
>> Wayne
>>
>> On 1/9/2018 10:39 AM, Jeff Young wrote:
>>> Hi Wayne,
>>>
>>> Well, the bug has a heat of 22, so it’s definitely not just me. ;)
>>>
>>> My change doesn’t alter the dragging or selecting behaviour.  All it
>>> does is keep an extraneous “Clarify Selection” menu from coming up
>>> (which I think all our users would consider a bug).  What we currently
>>> do in these situations is akin to popping up a “Clarify Selection” menu
>>> with one item in it every time you click on a unambiguous item.
>>>
>>> In the corner case all my change does is prevent us from asking: do you
>>> want to drag the corner of a and b, or do you want to drag the corner of
>>> b and a, when in fact the two have identical semantics).  Everything
>>> after the menu (no matter which item you click) is exactly the same.
>>>
>>> Same with U and I.  My change has no effect on what is selected, it just
>>> keeps us from asking: do you want to select the trivial connection
>>> containing a or do you want to select the trivial connection containing
>>> b, when in fact both a and b are on the /same/ trivial connection.
>>>  Again, everything after the menu (no matter which item you click) is
>>> exactly the same.
>>>
>>> Cheers,
>>> Jeff.
>>>
>>>> On 9 Jan 2018, at 15:27, Wayne Stambaugh <stambaughw@xxxxxxxxx
>>>> <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>> wrote:
>>>>
>>>> Jeff,
>>>>
>>>> Have actually confirmed that this is the desired behavior for this
>>>> outside of you own objectives?  I'm not saying that this is or isn't a
>>>> good idea but I personally don't drag trace corners around so I'm not
>>>> sure what the appropriate behavior should be.  You should get comments
>>>> from the dev list and users before you make a change like this.  As far
>>>> as pushing this to the dev repo, if it's not too invasive I will
>>>> consider it.  If it is a large change set, I would prefer that we hold
>>>> off until after the stable release.
>>>>
>>>> Thanks,
>>>>
>>>> Wayne
>>>>
>>>> On 1/8/2018 5:49 AM, Jeff Young wrote:
>>>>> Wayne, if I could get you to don that old project manager’s hat one
>>>>> more time:
>>>>>
>>>>> If we’re still weeks out from declaring an RC, I wanted to make one
>>>>> more plug for getting rid of the Clarify Selection dialog when
>>>>> dragging corners or using ‘U’ or ‘I’ over a corner[1].
>>>>>
>>>>> While it’s marked Wishlist, it seriously impacts productivity when
>>>>> editing tracks, and I think most users would consider it a bug
>>>>> (particularly in the corner case when dragging the corner is clearly
>>>>> moving both the tracks listed in the Clarify Selection menu).
>>>>>
>>>>> I’ve been running the patch for about a week now with no issues.
>>>>>
>>>>> Cheers,
>>>>> Jeff.
>>>>>
>>>>> [1] https://bugs.launchpad.net/kicad/+bug/1708869
>>>>> _______________________________________________
>>>>> Mailing list: https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>>>>> Unsubscribe : https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>>>>> More help   : https://help.launchpad.net/ListHelp <https://help.launchpad.net/ListHelp>
>>>>>
>>>>
>>>> _______________________________________________
>>>> Mailing list: https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>>>> Unsubscribe : https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>>>> More help   : https://help.launchpad.net/ListHelp <https://help.launchpad.net/ListHelp>
> 
> 
> 
> _______________________________________________
> 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
> 

From 706184575649c43fdcff35c0604f6d61f7cf865e Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Tue, 16 Jan 2018 11:50:12 +0100
Subject: [PATCH] Code formatting fixes for c7e5f0ab

---
 pcbnew/router/router_tool.cpp   | 10 +++++++++-
 pcbnew/tools/edit_tool.cpp      |  5 +++--
 pcbnew/tools/selection_tool.cpp | 14 +++++++++-----
 pcbnew/tools/selection_tool.h   |  2 +-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp
index bfcafc005..95f59137a 100644
--- a/pcbnew/router/router_tool.cpp
+++ b/pcbnew/router/router_tool.cpp
@@ -992,11 +992,13 @@ void ROUTER_TOOL::NeighboringSegmentFilter( const VECTOR2I& aPt, GENERAL_COLLECT
     // First make sure we've got something that *might* match.
     int vias = aCollector.CountType( PCB_VIA_T );
     int traces = aCollector.CountType( PCB_TRACE_T );
+
     if( vias > 1 || traces > 2 || vias + traces < 1 )
         return;
 
     // Fetch first TRACK (via or trace) as our reference
     TRACK* reference = nullptr;
+
     for( int i = 0; !reference && i < aCollector.GetCount(); i++ )
         reference = dynamic_cast<TRACK*>( aCollector[i] );
 
@@ -1004,6 +1006,7 @@ void ROUTER_TOOL::NeighboringSegmentFilter( const VECTOR2I& aPt, GENERAL_COLLECT
 
     wxPoint refPoint;
     STATUS_FLAGS flags = reference->IsPointOnEnds( wxPoint( aPt.x, aPt.y ), -1 );
+
     if( flags & STARTPOINT )
         refPoint = reference->GetStart();
     else if( flags & ENDPOINT )
@@ -1016,17 +1019,21 @@ void ROUTER_TOOL::NeighboringSegmentFilter( const VECTOR2I& aPt, GENERAL_COLLECT
     for( int i = 0; i < aCollector.GetCount(); i++ )
     {
         BOARD_ITEM* item = static_cast<BOARD_ITEM*>( aCollector[i] );
+
         if( item->Type() == PCB_MARKER_T )
             continue;
+
         TRACK* neighbor = dynamic_cast<TRACK*>( item );
+
         if( !neighbor || neighbor->GetNetCode() != refNet )
             return;
+
         if( neighbor->GetStart() != refPoint && neighbor->GetEnd() != refPoint )
             return;
     }
 
     // Selection meets criteria; trim it to the reference item.
-    for( int i = aCollector.GetCount()-1; i >= 0; i-- )
+    for( int i = aCollector.GetCount() - 1; i >= 0; i-- )
     {
         if( dynamic_cast<TRACK*>( aCollector[i] ) != reference )
             aCollector.Remove( i );
@@ -1042,6 +1049,7 @@ bool ROUTER_TOOL::CanInlineDrag()
     if( selection.Size() == 1 )
     {
         const BOARD_CONNECTED_ITEM* item = static_cast<const BOARD_CONNECTED_ITEM*>( selection.Front() );
+
         if( item->Type() == PCB_TRACE_T || item->Type() == PCB_VIA_T )
             return true;
     }
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 978eaa512..35baa77d1 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -1089,6 +1089,7 @@ void EDIT_TOOL::FootprintFilter( const VECTOR2I&, GENERAL_COLLECTOR& aCollector
     for( int i = aCollector.GetCount()-1; i >= 0; i-- )
     {
         BOARD_ITEM* item = static_cast<BOARD_ITEM*>( aCollector[i] );
+
         if( item->Type() != PCB_MODULE_T )
             aCollector.Remove( i );
     }
@@ -1097,7 +1098,7 @@ void EDIT_TOOL::FootprintFilter( const VECTOR2I&, GENERAL_COLLECTOR& aCollector
 
 int EDIT_TOOL::ExchangeFootprints( const TOOL_EVENT& aEvent )
 {
-    const auto& selection = m_selectionTool->RequestSelection(0, FootprintFilter);
+    const auto& selection = m_selectionTool->RequestSelection( 0, FootprintFilter );
 
     if( selection.Empty() )
         return 0;
@@ -1273,7 +1274,7 @@ bool EDIT_TOOL::updateModificationPoint( SELECTION& aSelection )
 
 int EDIT_TOOL::editFootprintInFpEditor( const TOOL_EVENT& aEvent )
 {
-    const auto& selection = m_selectionTool->RequestSelection(0, FootprintFilter);
+    const auto& selection = m_selectionTool->RequestSelection( 0, FootprintFilter );
 
     if( selection.Empty() )
         return 0;
diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp
index be7ba00c2..cbf8d62d1 100644
--- a/pcbnew/tools/selection_tool.cpp
+++ b/pcbnew/tools/selection_tool.cpp
@@ -365,6 +365,7 @@ SELECTION& SELECTION_TOOL::RequestSelection( int aFlags, CLIENT_SELECTION_FILTER
     {
         if( aFlags & SELECTION_FORCE_UNLOCK )
             m_locked = false;
+
         m_toolMgr->RunAction( PCB_ACTIONS::selectionCursor, true, aClientFilter );
         m_selection.SetIsHover( true );
         m_selection.ClearReferencePoint();
@@ -469,9 +470,8 @@ bool SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag,
     if( collector.GetCount() == 0 )
     {
         if( !m_additive && anyCollected )
-        {
             clearSelection();
-        }
+
         return false;
     }
 
@@ -497,6 +497,7 @@ bool SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag,
     }
 
     BOARD_ITEM* item = disambiguationMenu( &collector );
+
     if( item )
     {
         toggleSelection( item );
@@ -506,6 +507,7 @@ bool SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag,
     {
         if( aSelectionCancelledFlag )
             *aSelectionCancelledFlag = true;
+
         return false;
     }
 
@@ -811,7 +813,7 @@ void connectedTrackFilter( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector )
     /* Narrow the collection down to a single TRACK item for a trivial
      * connection, or multiple TRACK items for non-trivial connections.
      */
-    for( int i = aCollector.GetCount()-1; i >= 0; i-- )
+    for( int i = aCollector.GetCount() - 1; i >= 0; i-- )
     {
         if( !dynamic_cast<TRACK*>( aCollector[i] ) )
             aCollector.Remove( i );
@@ -832,6 +834,7 @@ int SELECTION_TOOL::selectConnection( const TOOL_EVENT& aEvent )
     for( auto item : selection )
     {
         TRACK* trackItem = dynamic_cast<TRACK*>( item );
+
         if( trackItem )
             selectAllItemsConnectedToTrack( *trackItem );
     }
@@ -851,9 +854,10 @@ void connectedItemFilter( const VECTOR2I&, GENERAL_COLLECTOR& aCollector )
      */
     std::set<int> representedNets;
 
-    for( int i = aCollector.GetCount()-1; i >= 0; i-- )
+    for( int i = aCollector.GetCount() - 1; i >= 0; i-- )
     {
         BOARD_CONNECTED_ITEM* item = dynamic_cast<BOARD_CONNECTED_ITEM*>( aCollector[i] );
+
         if( !item )
             aCollector.Remove( i );
         else if ( representedNets.count( item->GetNetCode() ) )
@@ -875,6 +879,7 @@ int SELECTION_TOOL::selectCopper( const TOOL_EVENT& aEvent )
     for( auto item : selection )
     {
         BOARD_CONNECTED_ITEM* connItem = dynamic_cast<BOARD_CONNECTED_ITEM*>( item );
+
         if( connItem )
             selectAllItemsConnectedToItem( *connItem );
     }
@@ -940,7 +945,6 @@ int SELECTION_TOOL::selectNet( const TOOL_EVENT& aEvent )
         if( item->IsConnected() )
         {
             auto& connItem = static_cast<BOARD_CONNECTED_ITEM&>( *item );
-
             selectAllItemsOnNet( connItem.GetNetCode() );
         }
     }
diff --git a/pcbnew/tools/selection_tool.h b/pcbnew/tools/selection_tool.h
index c96bd6a7d..c578ff96d 100644
--- a/pcbnew/tools/selection_tool.h
+++ b/pcbnew/tools/selection_tool.h
@@ -46,7 +46,7 @@ namespace KIGFX
     class GAL;
 }
 
-
+///> Selection filtering function signature
 typedef void (*CLIENT_SELECTION_FILTER)( const VECTOR2I&, GENERAL_COLLECTOR& );
 
 /**
-- 
2.13.3

Attachment: signature.asc
Description: OpenPGP digital signature