kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #37207
Re: [PATCH] RFC: toolbar button support for action plugins
I really like it!! Very cool work. Thank you.
Fabrizio
On Aug 22, 2018 1:29 PM, "Andrew Lutsenko" <anlutsenko@xxxxxxxxx> wrote:
Hi Seth,
I built the settings dialog for action plugins. You can reorder and
enable/disable buttons for each plugin individually.
Short demo:
https://i.imgur.com/33iJC7b.gif
Squashed patch is attached. I've tested it on win, debian8 and debian9.
If it's easier to review diff can be viewed here as well:
https://github.com/KiCad/kicad-source-mirror/compare/
master...qu1ck:plugin-icon
Also I've attached few dummy plugins to play with, 3 out of 4 have icons.
Let me know if you have any comments.
Thanks,
Andrew
On Fri, Aug 17, 2018 at 3:02 PM Andrew Lutsenko <anlutsenko@xxxxxxxxx>
wrote:
> Hi Seth,
>
> That makes sense. I will keep working on this feature and will ping this
> thread again once user configuration is implemented.
>
> Thanks,
> Andrew
>
> On Fri, Aug 17, 2018 at 10:03 AM Seth Hillbrand <seth@xxxxxxxxxxxxx>
> wrote:
>
>> Hi Andrew-
>>
>> I like the patch idea and your implementation approach is good.
>>
>> The coding style policy is located at https://kicad-source-
>> mirror.readthedocs.io/en/stable/Documentation/development/coding-style-
>> policy/ We're not totally consistent but we try to ensure any new code
>> follows it and clean up the old code as we go.
>>
>> On the errors, please don't throw an error to the user. It may be useful
>> to insert a wxLog() call that we could utilize in the future but that is
>> ignored for now.
>>
>> I'm opposed to merging patches that are partially complete. And I would
>> consider the lack of user control over this feature problematic. This is
>> not a judgement on the patch as you've implemented it. I really like the
>> functionality and think KiCad users will appreciate it as well. However, a
>> partially implemented feature creates the opportunity for problems down the
>> road that will distract developer time from the other tasks they are
>> working on. If you do not have the time to fully implement user control
>> over this feature (I completely understand competing time pressures), you
>> may consider opening a bug report and attaching your patch there for
>> interested future developers. Squashing the patchset for review is also a
>> good idea.
>>
>> Overall this looks really promising.
>>
>> Best-
>> Seth
>>
>> Am Do., 16. Aug. 2018 um 23:02 Uhr schrieb Andrew Lutsenko <
>> anlutsenko@xxxxxxxxx>:
>>
>>> Hi Seth
>>>
>>> I just checked out new preferences in pcbnew, looks much more organized
>>> than before.
>>> I totally can add a new tab there "pcbnew->Action plugins" and list the
>>> plugins there with option
>>> to remove toolbar icon. But that is a non-negligible amount of work.
>>> Will you hold off on merging
>>> my current patches until I implement that too?
>>> By default plugins will not show any buttons on toolbar, plugin writers
>>> will have to explicitly update
>>> their plugins and provide an icon for them to show up so you will not
>>> run into an issue with full
>>> toolbar for a while. See my screenshot in second email of the chain, it
>>> has 4 plugins but only
>>> 2 of them register with an icon and toolbar button.
>>>
>>> > headers get 1 space between function defs
>>> I tried to follow existing style in each file and didn't notice that
>>> it's not consistent across different files.
>>> action_plugin.h has two new lines between most functions but I can
>>> change it to one.
>>>
>>> What do you think about throwing an error to user if icon failed to
>>> load? Andrey Kuznetsov made a
>>> point that user can't do anything about it anyway. I agree that asking
>>> users to fix plugin icon declaration
>>> is a bit much and developer will be able to see that icon didn't load
>>> without the error too.
>>>
>>> Andrew
>>>
>>> On Thu, Aug 16, 2018 at 10:22 PM Seth Hillbrand <seth@xxxxxxxxxxxxx>
>>> wrote:
>>>
>>>> Hi Andrew-
>>>>
>>>> I like the idea. Aside from minor formatting (headers get 1 space
>>>> between function defs, need a space before the if block), the patch looks
>>>> good.
>>>>
>>>> However, I wouldn't want everything showing on my toolbar (speaking as
>>>> someone who has 10 plugins installed, 5 of which get regular use). I'd
>>>> prefer the option to be configurable. This should probably be in the
>>>> preferences pane that Jeff has been re-working.
>>>>
>>>> -Seth
>>>>
>>>> Am Do., 16. Aug. 2018 um 22:11 Uhr schrieb Andrew Lutsenko <
>>>> anlutsenko@xxxxxxxxx>:
>>>>
>>>>> Hi Clemens,
>>>>>
>>>>> See sample plugin attached. Extract it into kicad's
>>>>> share/scripting/plugins folder.
>>>>> One of other scanned directories that are documented in kicadplugins.i
>>>>> <https://github.com/KiCad/kicad-source-mirror/blob/6fdc5972f8431b4d5831a32649e67bfe20d05de8/scripting/kicadplugins.i#L180> should
>>>>> work too.
>>>>>
>>>>> Or are you asking to update docs in the repo?
>>>>> Documentation/development/pcbnew-plugins.md seems like the right
>>>>> place.
>>>>> I will update it once committers agree with the path I've chosen to
>>>>> implement this.
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Aug 16, 2018 at 4:48 AM Clemens Koller <cko@xxxxxxxxx> wrote:
>>>>>
>>>>>> Hello, Andrew!
>>>>>>
>>>>>> I am somehow missing the Python example you are giving in your patch.
>>>>>> Can you add this a simple example to the sources to get a basic/dummy
>>>>>> skeleton working right from scratch?
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Clemens
>>>>>>
>>>>>>
>>>>>> On 15/08/2018 11.31, Andrew Lutsenko wrote:
>>>>>> > Hi KiCad devs,
>>>>>> >
>>>>>> > I am proposing an addition to plugin system.
>>>>>> > Probably most will agree that menus suck. Toolbars suck less :)
>>>>>> >
>>>>>> > In my plugin I added a dirty hack to modify top toolbar from plugin
>>>>>> init code to add a button
>>>>>> > that calls plugins run() method. It is broken on linux X11 and is
>>>>>> not a sustainable way others
>>>>>> > can add buttons in their plugins. But having a button was quite
>>>>>> popular among users so I
>>>>>> > decided to implement this functionality directly in pcbnew.
>>>>>> >
>>>>>> > I introduced one more field plugin writers can define in defaults()
>>>>>> that contains path to png icon
>>>>>> > and if that string is not empty, pcbnew will attempt to load that
>>>>>> icon and add a button to top
>>>>>> > toolbar with action that calls the same run() method. I traced in
>>>>>> code how plugin action menu
>>>>>> > is generated and added similar logic for buttons.
>>>>>> >
>>>>>> > Here is how the result looks like:
>>>>>> >
>>>>>> > https://i.imgur.com/f3xg1FE.gif
>>>>>> >
>>>>>> > Sample dummy plugin __init__.py code:
>>>>>> >
>>>>>> > import os
>>>>>> > import pcbnew
>>>>>> > import wx
>>>>>> >
>>>>>> > class Plugin1(pcbnew.ActionPlugin):
>>>>>> >
>>>>>> > def defaults(self):
>>>>>> > self.name <http://self.name> = "Dummy Plugin 1"
>>>>>> > self.category = "Read PCB"
>>>>>> > self.description = ""
>>>>>> > self.icon_file_name = os.path.join(os.path.dirname(__file__),
>>>>>> 'icon.png')
>>>>>> >
>>>>>> > def Run(self):
>>>>>> > wx.MessageBox("Plugin 1")
>>>>>> >
>>>>>> > Plugin1().register()
>>>>>> >
>>>>>> > It's as simple as that.
>>>>>> >
>>>>>> > The patch is attached. It probably needs some error checking but
>>>>>> seems to be working great.
>>>>>> > Tested in win64 so far.
>>>>>> > I'm open to suggestions on how to get it to good state, I will also
>>>>>> test on linux asap.
>>>>>> >
>>>>>> > Regards,
>>>>>> > Andrew
>>>>>> >
>>>>>> > _______________________________________________
>>>>>> > 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
>>>>>> >
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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
>>>>>>
>>>>> _______________________________________________
>>>>> 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
>>>>>
>>>>
_______________________________________________
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