← Back to team overview

kicad-developers team mailing list archive

Re: [Patch] Add an option to select a reference point and an anchor in pcbnew move exactly dialog

 

Robbert,

Please disregard this comment.  I was confusing the drill/place file
origin with the cursor offset (user) origin.  This does raise two
issues.  The term "User Origin" is a bit misleading.  You might want to
use "Cursor Offset Origin" or at least provide a tool tip to clarify the
"User Origin" choice.  It would also be useful to add another "Move
relative to:" entry for moves relative to the drill origin.

Cheers,

Wayne

On 3/21/2017 11:51 AM, Wayne Stambaugh wrote:
> I did some quick testing on this and the move relative the user origin
> always moves relative to the page origin.  Everything else appears to
> work as expected.
> 
> On 3/20/2017 4:31 PM, Robbert Lagerweij wrote:
>> Just a gentle bump on this patch. I have rebased it on master today. 
>>
>> As said, I'm more than happy to rip out the legacy canvas stuff if this is the preference. Also happy to take on board any other comments or suggestions for changes.
>>
>> Kind regards,
>>
>> Robbert
>>
>>
>> From: Robbert Lagerweij <rlagerweij@xxxxxxxxxxx>
>> Sent: Tuesday, March 7, 2017 11:16 PM
>> To: John Beard
>> Cc: Tomasz Wlostowski; KiCad Developers
>> Subject: Re: [Kicad-developers] [Patch] Add an option to select a reference point and an anchor in pcbnew move exactly dialog
>>     
>> Well, I finally had some time to look at improving this patch.
>>
>> This version has the coding style policy issue fixed ( and uncrustified ). Inspired by Thomas' comment I removed some duplication in the selection_tool code.
>> Given that I haven't received any further comments other than those of John, I've left the duplication between GAL and Legacy in.
>>
>> Please let me know if anyone has any further suggestions to improve this.
>>
>> Robbert
>>
>> From: John Beard <john.j.beard@xxxxxxxxx>
>> Sent: Tuesday, February 28, 2017 2:08 PM
>> To: Robbert Lagerweij
>> Cc: Tomasz Wlostowski; KiCad Developers
>> Subject: Re: [Kicad-developers] [Patch] Add an option to select a reference point and an anchor in pcbnew move exactly dialog
>>     
>> On Tue, Feb 28, 2017 at 9:00 PM, Robbert Lagerweij
>> <rlagerweij@xxxxxxxxxxx> wrote:
>>> Hi Thomas,
>>>
>>> Thank you for your feedback. The duplication of code is indeed not how I
>>> would usually approach this but I thought it the lesser of two evils given
>>> the fact that the legacy stuff will most likely be deprecated immediately
>>> after the 5.0 release. This means that long term maintainability is not
>>> really affected since there will be limited chance of structural changes
>>> needed to that part of the code (presumably only 5.x bug fixes) .
>>
>> Since the work is done already in Legacy, I'd vote to leave it in.
>> Duplicating code is paradoxically probably the cleanest way, as when
>> legacy gets the chop, the GAL code will be unaffected. Combining the
>> code is probably more likely to need tidying up in future that isn't
>> just deleting one of the call sites.
>>
>>> If we go the GAL only route, since legacy and GAL use the same dialog, I
>>> could either create a new dialog which we only use in GAL or add additional
>>> logic to disable/hide the functionality in legacy.
>>
>> I think a new dialog is overkill and not worth the effort since the
>> legacy work is done. At most, a constructor parameter to hide relevant
>> UI controls would suffice, and be easy to rip out later.
>>
>> 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
>>


References