That was a really good refactor, now the code seems much cleaner. I have just committed your patches. Once again, thank you Jon!
Regards,
Orson
On 02/20/2017 07:12 PM, Jon Evans wrote:
> This time with the patch attached :)
>
> On Mon, Feb 20, 2017 at 1:12 PM, Jon Evans <j...@craftyjon.com> wrote:
>
>> Hi Orson,
>>
>> I've attached a follow-up patch that moves all TOOL_ACTIONs out of
>> pcb_actions.cpp and creates a COMMON_TOOLS class for storing
>> cross-application tools. I was not able to move all zoom/grid tools
>> because of dependencies on pcbnew that need to be resolved -- I did not
>> want to take on the refactoring needed to fix this in this patch, but I
>> plan on looking at it in the near future.
>>
>> Best,
>> Jon
>>
>> On Mon, Feb 20, 2017 at 11:13 AM, Maciej Sumiński <maciej.sumin...@cern.ch
>>> wrote:
>>
>>> Thank you John, we really appreciate your efforts.
>>>
>>> Regards,
>>> Orson
>>>
>>> On 02/20/2017 03:50 PM, Jon Evans wrote:
>>>> Hi Orson,
>>>>
>>>> I can definitely pull the pcb_actions into their respective files, I
>>> will
>>>> do that and send another patch.
>>>>
>>>> Best,
>>>> Jon
>>>>
>>>> On Mon, Feb 20, 2017 at 4:25 AM, John Beard <john.j.be...@gmail.com>
>>> wrote:
>>>>
>>>>> HI Orson,
>>>>>
>>>>> I think that sounds like a sensible idea. Having a huge central list
>>>>> of actions has a bit of a code smell for me, as it's a big header than
>>>>> then needs including everywhere. Smaller lists that are included along
>>>>> with their tool's headers (if needed), or even actions that are
>>>>> totally hidden in implementations for when the action only works
>>>>> during an interactive tool feels much nicer.
>>>>>
>>>>> The file now called common_actions.cpp (maybe pcb_actions.cpp or
>>>>> something in future) would need to include most of these headers so it
>>>>> can still map legacy event IDs, but that's how it should be - a file
>>>>> that needs lots, includes lots.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> John
>>>>>
>>>>>
>>>>> On Mon, Feb 20, 2017 at 4:58 PM, Maciej Sumiński
>>>>> <maciej.sumin...@cern.ch> wrote:
>>>>>> Hi Jon,
>>>>>>
>>>>>> I see the point of your patch, as COMMON_ACTIONS are now a bit
>>> misused.
>>>>>> They should not keep majority of the TOOL_ACTIONs, as many of them are
>>>>>> pcbnew specific, but there are still actions that will be shared with
>>>>>> other applications (e.g. zoom & grid control, move/rotate/flip).
>>>>>>
>>>>>> For some time I was also wondering whether it would not be better to
>>>>>> move the actions to their corresponding tools, as is done e.g. in
>>>>>> pcbnew/router/router_tool.cpp (ACT_* objects), and leave only truly
>>>>>> generic actions in {COMMON,PCB}_ACTIONS.
>>>>>>
>>>>>> What do you think about splitting the current set to PCB_ACTIONS and
>>>>>> COMMON_ACTIONS, perhaps moving some of them to the tools source files?
>>>>>>
>>>>>> Regards,
>>>>>> Orson
>>>>>>
>>>>>> On 02/17/2017 04:56 AM, Jon Evans wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> More preparation for GerbView GAL port: this patch pulls a virtual
>>>>> ACTIONS
>>>>>>> class out of pcbnew and renames the COMMON_ACTIONS to PCB_ACTIONS for
>>>>>>> clarity.
>>>>>>>
>>>>>>> Best,
>>>>>>> Jon
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>> Post to : kicad-developers@lists.launchpad.net
>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>> More help : https://help.launchpad.net/ListHelp
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>> Post to : kicad-developers@lists.launchpad.net
>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>> More help : https://help.launchpad.net/ListHelp
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>> Post to : kicad-developers@lists.launchpad.net
>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>> More help : https://help.launchpad.net/ListHelp
>>>>>
>>>>
>>>
>>>
>>>
>>
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
