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 <[email protected]> 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 <[email protected]> > 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 > >> <[email protected]> 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 : [email protected] > >>>> Unsubscribe : https://launchpad.net/~kicad-developers > >>>> More help : https://help.launchpad.net/ListHelp > >>>> > >>> > >>> > >>> > >>> _______________________________________________ > >>> Mailing list: https://launchpad.net/~kicad-developers > >>> Post to : [email protected] > >>> Unsubscribe : https://launchpad.net/~kicad-developers > >>> More help : https://help.launchpad.net/ListHelp > >>> > >> > >> _______________________________________________ > >> Mailing list: https://launchpad.net/~kicad-developers > >> Post to : [email protected] > >> Unsubscribe : https://launchpad.net/~kicad-developers > >> More help : https://help.launchpad.net/ListHelp > >> > > > > >
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
