Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Wayne Stambaugh]

Andrew,

I merged your patch into the development branch.  Thank you for your
contribution.

Cheers,

Wayne

On 8/27/2018 12:26 AM, Andrew Lutsenko wrote:
> Awesome!
> 
> I updated the docs for plugin writers. Please see attached file.
> 
> On Sun, Aug 26, 2018 at 7:48 PM Seth Hillbrand  > wrote:
> 
> Merged.
> 
> Thank you for your contribution to KiCad!  
> 
> -Seth
> 
> Am So., 26. Aug. 2018 um 13:27 Uhr schrieb Andrew Lutsenko
> mailto:anlutse...@gmail.com>>:
> 
> Thanks all for the review.
> 
> Now that proposal is approved I'm not sure what next steps are.
> Only docs I could find on launchpad are about bazaar.
> But I would guess commit has to be manually merged. I attached
> approved revision of the patch here.
> 
> On Thu, Aug 23, 2018 at 1:50 PM Seth Hillbrand
> mailto:s...@hillbrand.org>> wrote:
> 
> 
> 
> Am Do., 23. Aug. 2018 um 12:38 Uhr schrieb Andrew Lutsenko
> mailto:anlutse...@gmail.com>>:
> 
> Thanks, I opened merge request on launchpad:
> 
> https://code.launchpad.net/~qu1ck/kicad/+git/kicad/+merge/353677
> 
> 
> Excellent, thanks.  I've added Jeff for a second set of eyes
> to look for possible Mac issues.  I've added a few comments
> for formatting (they probably sound pedantic by now)and a
> couple suggestions.  Once my comments are addressed, it
> looks good to me.
> 
>  
> 
> > ACTION_PLUGINS::GetActionButton.  Takes an integer and
> picks the element from the actions list vector.  But
> std::vector[] doesn't provide bounds checking.  You
> could use at() and catch the out_of_range error.
> 
> That method is not used. I only added it because there
> was ::GetActionMenu, which has the same issue you
> mentioned. It is also unused, maybe I should remove both.
> 
> 
> Removing both is fine.  If we do decide to leave them in for
> future implementation, they will need to be safe for callers.
> 
> -Seth
> 
> 
> On Thu, Aug 23, 2018 at 10:56 AM Seth Hillbrand
> mailto:s...@hillbrand.org>> wrote:
> 
> 
> 
> Am Do., 23. Aug. 2018 um 00:51 Uhr schrieb Andrew
> Lutsenko  >:
> 
> I fixed whitespace issue and formatting (at
> least what I caught).
> Fixed a bug that would not load plugin list
> properly if one plugin was removed and another
> was added at the same time.
> Also made plugin buttons scale same as the main
> buttons.
> 
> Please see updated patch attached.
> 
> > You should protect array dereference indices
> to prevent overflow.  It would be a good idea to
> define a specific "no icon" index (-1 maybe?  or
> 0) that returns a default icon (0) or prevents
> an icon from being loaded (-1).
> 
> I am not sure what place in code are you
> referring to here. Nowhere do I reference icons
> by index. Icons are fields of action plugins and
> I get them from a map, not a vector.
> 
> 
> ACTION_PLUGINS::GetActionButton.  Takes an integer
> and picks the element from the actions list vector. 
> But std::vector[] doesn't provide bounds checking. 
> You could use at() and catch the out_of_range error.
> 
>  
> 
> Would you prefer if I opened a PR on github? I
> know it won't be merged, just to make exchanging
> comments on code easier during review.
> 
> 
> No, I'd prefer not to split the conversation. 
> However, you can make a branch on launchpad and then
> make a merge proposal.  That should be the same
> workflow and keeps information on our primary
> platform. (see attached screenshot for details on
> where this button is hidden)
> 
> -S
> 
>  
> 
> 
> Andrew
> 
> On Wed, Aug 22, 2018 at 2:03 PM Seth Hillbrand
> mailto:s...@hillbrand.org>>
> wrote:
> 
> Thanks Andrew, I'll have a chance to test
> this in detail this 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

Awesome!

I updated the docs for plugin writers. Please see attached file.

On Sun, Aug 26, 2018 at 7:48 PM Seth Hillbrand  wrote:

> Merged.
>
> Thank you for your contribution to KiCad!
>
> -Seth
>
> Am So., 26. Aug. 2018 um 13:27 Uhr schrieb Andrew Lutsenko <
> anlutse...@gmail.com>:
>
>> Thanks all for the review.
>>
>> Now that proposal is approved I'm not sure what next steps are. Only docs
>> I could find on launchpad are about bazaar.
>> But I would guess commit has to be manually merged. I attached approved
>> revision of the patch here.
>>
>> On Thu, Aug 23, 2018 at 1:50 PM Seth Hillbrand 
>> wrote:
>>
>>>
>>>
>>> Am Do., 23. Aug. 2018 um 12:38 Uhr schrieb Andrew Lutsenko <
>>> anlutse...@gmail.com>:
>>>
 Thanks, I opened merge request on launchpad:
 https://code.launchpad.net/~qu1ck/kicad/+git/kicad/+merge/353677

>>>
>>> Excellent, thanks.  I've added Jeff for a second set of eyes to look for
>>> possible Mac issues.  I've added a few comments for formatting (they
>>> probably sound pedantic by now)and a couple suggestions.  Once my comments
>>> are addressed, it looks good to me.
>>>
>>>
>>>
 > ACTION_PLUGINS::GetActionButton.  Takes an integer and picks the
 element from the actions list vector.  But std::vector[] doesn't provide
 bounds checking.  You could use at() and catch the out_of_range error.

 That method is not used. I only added it because there was
 ::GetActionMenu, which has the same issue you mentioned. It is also unused,
 maybe I should remove both.

>>>
>>> Removing both is fine.  If we do decide to leave them in for future
>>> implementation, they will need to be safe for callers.
>>>
>>> -Seth
>>>

 On Thu, Aug 23, 2018 at 10:56 AM Seth Hillbrand 
 wrote:

>
>
> Am Do., 23. Aug. 2018 um 00:51 Uhr schrieb Andrew Lutsenko <
> anlutse...@gmail.com>:
>
>> I fixed whitespace issue and formatting (at least what I caught).
>> Fixed a bug that would not load plugin list properly if one plugin
>> was removed and another was added at the same time.
>> Also made plugin buttons scale same as the main buttons.
>>
>> Please see updated patch attached.
>>
>> > You should protect array dereference indices to prevent overflow.
>> It would be a good idea to define a specific "no icon" index (-1 maybe?  
>> or
>> 0) that returns a default icon (0) or prevents an icon from being loaded
>> (-1).
>>
>> I am not sure what place in code are you referring to here. Nowhere
>> do I reference icons by index. Icons are fields of action plugins and I 
>> get
>> them from a map, not a vector.
>>
>
> ACTION_PLUGINS::GetActionButton.  Takes an integer and picks the
> element from the actions list vector.  But std::vector[] doesn't provide
> bounds checking.  You could use at() and catch the out_of_range error.
>
>
>
>> Would you prefer if I opened a PR on github? I know it won't be
>> merged, just to make exchanging comments on code easier during review.
>>
>
> No, I'd prefer not to split the conversation.  However, you can make a
> branch on launchpad and then make a merge proposal.  That should be the
> same workflow and keeps information on our primary platform. (see attached
> screenshot for details on where this button is hidden)
>
> -S
>
>
>
>>
>> Andrew
>>
>> On Wed, Aug 22, 2018 at 2:03 PM Seth Hillbrand 
>> wrote:
>>
>>> Thanks Andrew, I'll have a chance to test this in detail this
>>> weekend.  The movie looks really nice.
>>>
>>> Short comments:
>>>
>>> - You should protect array dereference indices to prevent overflow.
>>> It would be a good idea to define a specific "no icon" index (-1 maybe? 
>>>  or
>>> 0) that returns a default icon (0) or prevents an icon from being loaded
>>> (-1).
>>>
>>> - It looks like your editor doesn't automatically clear trailing
>>> whitespace at the end of lines.  Can you check if there's an option in 
>>> it
>>> that does that for you?
>>>
>>> - There are a couple small spacing errors (single line between
>>> function defs, space before closing parens)
>>>
>>> Overall, excellent work!  This feels like a nice addition for people
>>> who regularly use plugins.
>>>
>>> -Seth
>>>
>>> Am Mi., 22. Aug. 2018 um 04:28 Uhr schrieb Andrew Lutsenko <
>>> anlutse...@gmail.com>:
>>>
 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:

 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Seth Hillbrand]

Merged.

Thank you for your contribution to KiCad!

-Seth

Am So., 26. Aug. 2018 um 13:27 Uhr schrieb Andrew Lutsenko <
anlutse...@gmail.com>:

> Thanks all for the review.
>
> Now that proposal is approved I'm not sure what next steps are. Only docs
> I could find on launchpad are about bazaar.
> But I would guess commit has to be manually merged. I attached approved
> revision of the patch here.
>
> On Thu, Aug 23, 2018 at 1:50 PM Seth Hillbrand  wrote:
>
>>
>>
>> Am Do., 23. Aug. 2018 um 12:38 Uhr schrieb Andrew Lutsenko <
>> anlutse...@gmail.com>:
>>
>>> Thanks, I opened merge request on launchpad:
>>> https://code.launchpad.net/~qu1ck/kicad/+git/kicad/+merge/353677
>>>
>>
>> Excellent, thanks.  I've added Jeff for a second set of eyes to look for
>> possible Mac issues.  I've added a few comments for formatting (they
>> probably sound pedantic by now)and a couple suggestions.  Once my comments
>> are addressed, it looks good to me.
>>
>>
>>
>>> > ACTION_PLUGINS::GetActionButton.  Takes an integer and picks the
>>> element from the actions list vector.  But std::vector[] doesn't provide
>>> bounds checking.  You could use at() and catch the out_of_range error.
>>>
>>> That method is not used. I only added it because there was
>>> ::GetActionMenu, which has the same issue you mentioned. It is also unused,
>>> maybe I should remove both.
>>>
>>
>> Removing both is fine.  If we do decide to leave them in for future
>> implementation, they will need to be safe for callers.
>>
>> -Seth
>>
>>>
>>> On Thu, Aug 23, 2018 at 10:56 AM Seth Hillbrand 
>>> wrote:
>>>


 Am Do., 23. Aug. 2018 um 00:51 Uhr schrieb Andrew Lutsenko <
 anlutse...@gmail.com>:

> I fixed whitespace issue and formatting (at least what I caught).
> Fixed a bug that would not load plugin list properly if one plugin was
> removed and another was added at the same time.
> Also made plugin buttons scale same as the main buttons.
>
> Please see updated patch attached.
>
> > You should protect array dereference indices to prevent overflow.
> It would be a good idea to define a specific "no icon" index (-1 maybe?  
> or
> 0) that returns a default icon (0) or prevents an icon from being loaded
> (-1).
>
> I am not sure what place in code are you referring to here. Nowhere do
> I reference icons by index. Icons are fields of action plugins and I get
> them from a map, not a vector.
>

 ACTION_PLUGINS::GetActionButton.  Takes an integer and picks the
 element from the actions list vector.  But std::vector[] doesn't provide
 bounds checking.  You could use at() and catch the out_of_range error.



> Would you prefer if I opened a PR on github? I know it won't be
> merged, just to make exchanging comments on code easier during review.
>

 No, I'd prefer not to split the conversation.  However, you can make a
 branch on launchpad and then make a merge proposal.  That should be the
 same workflow and keeps information on our primary platform. (see attached
 screenshot for details on where this button is hidden)

 -S



>
> Andrew
>
> On Wed, Aug 22, 2018 at 2:03 PM Seth Hillbrand 
> wrote:
>
>> Thanks Andrew, I'll have a chance to test this in detail this
>> weekend.  The movie looks really nice.
>>
>> Short comments:
>>
>> - You should protect array dereference indices to prevent overflow.
>> It would be a good idea to define a specific "no icon" index (-1 maybe?  
>> or
>> 0) that returns a default icon (0) or prevents an icon from being loaded
>> (-1).
>>
>> - It looks like your editor doesn't automatically clear trailing
>> whitespace at the end of lines.  Can you check if there's an option in it
>> that does that for you?
>>
>> - There are a couple small spacing errors (single line between
>> function defs, space before closing parens)
>>
>> Overall, excellent work!  This feels like a nice addition for people
>> who regularly use plugins.
>>
>> -Seth
>>
>> Am Mi., 22. Aug. 2018 um 04:28 Uhr schrieb Andrew Lutsenko <
>> anlutse...@gmail.com>:
>>
>>> 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 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Rene Pöschl]

One can however archive a repo. (We did that with all the old library 
repos.)


I am not sure if the automatic synchronization with the lauchpad code 
would still work for archived repos.


On 23/08/18 20:31, José Ignacio wrote:

Pull requests cannot be disabled on Github

On Thu, Aug 23, 2018 at 1:26 PM, Carsten Schoenert 
wrote:


Hello Wayne,

Am 23.08.18 um 20:01 schrieb Wayne Stambaugh:

We do not use github for kicad source development.  It is merely a
mirror for user convenience.  Please do not open any issues on github
for the kicad source mirror.

I'd suggest to disable all these kind of features on GitHub then and
mention this circumstance more in detail in the project description on GH.

--
Regards
Carsten Schoenert

___
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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Wayne Stambaugh]

On 08/23/2018 02:46 PM, Carsten Schoenert wrote:
> Am 23.08.18 um 20:31 schrieb José Ignacio:
>> Pull requests cannot be disabled on Github
> 
> That's unfortunately true, but there is a "plugin" available for
> disabling pull requests.
> 
> http://nopullrequests.com/
> 

Thanks for the tip but I'm going to hold off on this for the time being.
 It hasn't been a huge problem just yet.

___
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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

Thanks, I opened merge request on launchpad:
https://code.launchpad.net/~qu1ck/kicad/+git/kicad/+merge/353677

> ACTION_PLUGINS::GetActionButton.  Takes an integer and picks the element
from the actions list vector.  But std::vector[] doesn't provide bounds
checking.  You could use at() and catch the out_of_range error.

That method is not used. I only added it because there was ::GetActionMenu,
which has the same issue you mentioned. It is also unused, maybe I should
remove both.

On Thu, Aug 23, 2018 at 10:56 AM Seth Hillbrand  wrote:

>
>
> Am Do., 23. Aug. 2018 um 00:51 Uhr schrieb Andrew Lutsenko <
> anlutse...@gmail.com>:
>
>> I fixed whitespace issue and formatting (at least what I caught).
>> Fixed a bug that would not load plugin list properly if one plugin was
>> removed and another was added at the same time.
>> Also made plugin buttons scale same as the main buttons.
>>
>> Please see updated patch attached.
>>
>> > You should protect array dereference indices to prevent overflow.  It
>> would be a good idea to define a specific "no icon" index (-1 maybe?  or 0)
>> that returns a default icon (0) or prevents an icon from being loaded (-1).
>>
>> I am not sure what place in code are you referring to here. Nowhere do I
>> reference icons by index. Icons are fields of action plugins and I get them
>> from a map, not a vector.
>>
>
> ACTION_PLUGINS::GetActionButton.  Takes an integer and picks the element
> from the actions list vector.  But std::vector[] doesn't provide bounds
> checking.  You could use at() and catch the out_of_range error.
>
>
>
>> Would you prefer if I opened a PR on github? I know it won't be merged,
>> just to make exchanging comments on code easier during review.
>>
>
> No, I'd prefer not to split the conversation.  However, you can make a
> branch on launchpad and then make a merge proposal.  That should be the
> same workflow and keeps information on our primary platform. (see attached
> screenshot for details on where this button is hidden)
>
> -S
>
>
>
>>
>> Andrew
>>
>> On Wed, Aug 22, 2018 at 2:03 PM Seth Hillbrand 
>> wrote:
>>
>>> Thanks Andrew, I'll have a chance to test this in detail this weekend.
>>> The movie looks really nice.
>>>
>>> Short comments:
>>>
>>> - You should protect array dereference indices to prevent overflow.  It
>>> would be a good idea to define a specific "no icon" index (-1 maybe?  or 0)
>>> that returns a default icon (0) or prevents an icon from being loaded (-1).
>>>
>>> - It looks like your editor doesn't automatically clear trailing
>>> whitespace at the end of lines.  Can you check if there's an option in it
>>> that does that for you?
>>>
>>> - There are a couple small spacing errors (single line between function
>>> defs, space before closing parens)
>>>
>>> Overall, excellent work!  This feels like a nice addition for people who
>>> regularly use plugins.
>>>
>>> -Seth
>>>
>>> Am Mi., 22. Aug. 2018 um 04:28 Uhr schrieb Andrew Lutsenko <
>>> anlutse...@gmail.com>:
>>>
 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 
 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 
> 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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Carsten Schoenert]

Am 23.08.18 um 20:31 schrieb José Ignacio:
> Pull requests cannot be disabled on Github

That's unfortunately true, but there is a "plugin" available for
disabling pull requests.

http://nopullrequests.com/

-- 
Regards
Carsten Schoenert

___
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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[José Ignacio]

Pull requests cannot be disabled on Github

On Thu, Aug 23, 2018 at 1:26 PM, Carsten Schoenert 
wrote:

> Hello Wayne,
>
> Am 23.08.18 um 20:01 schrieb Wayne Stambaugh:
> > We do not use github for kicad source development.  It is merely a
> > mirror for user convenience.  Please do not open any issues on github
> > for the kicad source mirror.
>
> I'd suggest to disable all these kind of features on GitHub then and
> mention this circumstance more in detail in the project description on GH.
>
> --
> Regards
> Carsten Schoenert
>
> ___
> 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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Carsten Schoenert]

Hello Wayne,

Am 23.08.18 um 20:01 schrieb Wayne Stambaugh:
> We do not use github for kicad source development.  It is merely a
> mirror for user convenience.  Please do not open any issues on github
> for the kicad source mirror.

I'd suggest to disable all these kind of features on GitHub then and
mention this circumstance more in detail in the project description on GH.

-- 
Regards
Carsten Schoenert

___
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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Wayne Stambaugh]

On 8/23/2018 3:50 AM, Andrew Lutsenko wrote:
> I fixed whitespace issue and formatting (at least what I caught).
> Fixed a bug that would not load plugin list properly if one plugin was
> removed and another was added at the same time.
> Also made plugin buttons scale same as the main buttons.
> 
> Please see updated patch attached.
> 
>> You should protect array dereference indices to prevent overflow.  It
> would be a good idea to define a specific "no icon" index (-1 maybe?  or
> 0) that returns a default icon (0) or prevents an icon from being loaded
> (-1).
> 
> I am not sure what place in code are you referring to here. Nowhere do I
> reference icons by index. Icons are fields of action plugins and I get
> them from a map, not a vector.
> 
> Would you prefer if I opened a PR on github? I know it won't be merged,
> just to make exchanging comments on code easier during review.


We do not use github for kicad source development.  It is merely a
mirror for user convenience.  Please do not open any issues on github
for the kicad source mirror.

Cheers,

Wayne

> 
> Andrew
> 
> On Wed, Aug 22, 2018 at 2:03 PM Seth Hillbrand  > wrote:
> 
> Thanks Andrew, I'll have a chance to test this in detail this
> weekend.  The movie looks really nice.
> 
> Short comments: 
> 
> - You should protect array dereference indices to prevent overflow. 
> It would be a good idea to define a specific "no icon" index (-1
> maybe?  or 0) that returns a default icon (0) or prevents an icon
> from being loaded (-1).
> 
> - It looks like your editor doesn't automatically clear trailing
> whitespace at the end of lines.  Can you check if there's an option
> in it that does that for you?
> 
> - There are a couple small spacing errors (single line between
> function defs, space before closing parens)
> 
> Overall, excellent work!  This feels like a nice addition for people
> who regularly use plugins.
> 
> -Seth
> 
> Am Mi., 22. Aug. 2018 um 04:28 Uhr schrieb Andrew Lutsenko
> mailto:anlutse...@gmail.com>>:
> 
> 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
> mailto:anlutse...@gmail.com>> 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
> mailto:s...@hillbrand.org>> 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.  
> 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Seth Hillbrand]

Am Do., 23. Aug. 2018 um 00:51 Uhr schrieb Andrew Lutsenko <
anlutse...@gmail.com>:

> I fixed whitespace issue and formatting (at least what I caught).
> Fixed a bug that would not load plugin list properly if one plugin was
> removed and another was added at the same time.
> Also made plugin buttons scale same as the main buttons.
>
> Please see updated patch attached.
>
> > You should protect array dereference indices to prevent overflow.  It
> would be a good idea to define a specific "no icon" index (-1 maybe?  or 0)
> that returns a default icon (0) or prevents an icon from being loaded (-1).
>
> I am not sure what place in code are you referring to here. Nowhere do I
> reference icons by index. Icons are fields of action plugins and I get them
> from a map, not a vector.
>

ACTION_PLUGINS::GetActionButton.  Takes an integer and picks the element
from the actions list vector.  But std::vector[] doesn't provide bounds
checking.  You could use at() and catch the out_of_range error.



> Would you prefer if I opened a PR on github? I know it won't be merged,
> just to make exchanging comments on code easier during review.
>

No, I'd prefer not to split the conversation.  However, you can make a
branch on launchpad and then make a merge proposal.  That should be the
same workflow and keeps information on our primary platform. (see attached
screenshot for details on where this button is hidden)

-S



>
> Andrew
>
> On Wed, Aug 22, 2018 at 2:03 PM Seth Hillbrand  wrote:
>
>> Thanks Andrew, I'll have a chance to test this in detail this weekend.
>> The movie looks really nice.
>>
>> Short comments:
>>
>> - You should protect array dereference indices to prevent overflow.  It
>> would be a good idea to define a specific "no icon" index (-1 maybe?  or 0)
>> that returns a default icon (0) or prevents an icon from being loaded (-1).
>>
>> - It looks like your editor doesn't automatically clear trailing
>> whitespace at the end of lines.  Can you check if there's an option in it
>> that does that for you?
>>
>> - There are a couple small spacing errors (single line between function
>> defs, space before closing parens)
>>
>> Overall, excellent work!  This feels like a nice addition for people who
>> regularly use plugins.
>>
>> -Seth
>>
>> Am Mi., 22. Aug. 2018 um 04:28 Uhr schrieb Andrew Lutsenko <
>> anlutse...@gmail.com>:
>>
>>> 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 
>>> 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 
 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 <
> anlutse...@gmail.com>:
>
>> Hi Seth
>>
>> I just checked out new preferences in pcbnew, looks much more
>> organized than before.
>> I totally can add a new 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

I fixed whitespace issue and formatting (at least what I caught).
Fixed a bug that would not load plugin list properly if one plugin was
removed and another was added at the same time.
Also made plugin buttons scale same as the main buttons.

Please see updated patch attached.

> You should protect array dereference indices to prevent overflow.  It
would be a good idea to define a specific "no icon" index (-1 maybe?  or 0)
that returns a default icon (0) or prevents an icon from being loaded (-1).

I am not sure what place in code are you referring to here. Nowhere do I
reference icons by index. Icons are fields of action plugins and I get them
from a map, not a vector.

Would you prefer if I opened a PR on github? I know it won't be merged,
just to make exchanging comments on code easier during review.

Andrew

On Wed, Aug 22, 2018 at 2:03 PM Seth Hillbrand  wrote:

> Thanks Andrew, I'll have a chance to test this in detail this weekend.
> The movie looks really nice.
>
> Short comments:
>
> - You should protect array dereference indices to prevent overflow.  It
> would be a good idea to define a specific "no icon" index (-1 maybe?  or 0)
> that returns a default icon (0) or prevents an icon from being loaded (-1).
>
> - It looks like your editor doesn't automatically clear trailing
> whitespace at the end of lines.  Can you check if there's an option in it
> that does that for you?
>
> - There are a couple small spacing errors (single line between function
> defs, space before closing parens)
>
> Overall, excellent work!  This feels like a nice addition for people who
> regularly use plugins.
>
> -Seth
>
> Am Mi., 22. Aug. 2018 um 04:28 Uhr schrieb Andrew Lutsenko <
> anlutse...@gmail.com>:
>
>> 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 
>> 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 
>>> 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 <
 anlutse...@gmail.com>:

> 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 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Seth Hillbrand]

Thanks Andrew, I'll have a chance to test this in detail this weekend.  The
movie looks really nice.

Short comments:

- You should protect array dereference indices to prevent overflow.  It
would be a good idea to define a specific "no icon" index (-1 maybe?  or 0)
that returns a default icon (0) or prevents an icon from being loaded (-1).

- It looks like your editor doesn't automatically clear trailing whitespace
at the end of lines.  Can you check if there's an option in it that does
that for you?

- There are a couple small spacing errors (single line between function
defs, space before closing parens)

Overall, excellent work!  This feels like a nice addition for people who
regularly use plugins.

-Seth

Am Mi., 22. Aug. 2018 um 04:28 Uhr schrieb Andrew Lutsenko <
anlutse...@gmail.com>:

> 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 
> 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 
>> 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 <
>>> anlutse...@gmail.com>:
>>>
 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 
 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 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Fabrizio Tappero]

I really like it!! Very cool work. Thank you.

Fabrizio

On Aug 22, 2018 1:29 PM, "Andrew Lutsenko"  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 
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 
> 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 <
>> anlutse...@gmail.com>:
>>
>>> 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 
>>> 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 <
 anlutse...@gmail.com>:

> 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
> 
>  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.
>

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

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 
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 
> 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 <
>> anlutse...@gmail.com>:
>>
>>> 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 
>>> 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 <
 anlutse...@gmail.com>:

> 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
> 
>  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  wrote:
>
>> Hello,  Andrew!
>>
>> I am 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

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  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 <
> anlutse...@gmail.com>:
>
>> 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 
>> 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 <
>>> anlutse...@gmail.com>:
>>>
 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
 
  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  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 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Seth Hillbrand]

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 <
anlutse...@gmail.com>:

> 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 
> 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 <
>> anlutse...@gmail.com>:
>>
>>> 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
>>> 
>>>  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  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 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

I found and fixed couple compilation issues and also implemented option to
not show toolbar icon,
just have the icon in menus. (Not user configurable, just for plugin
writers, another attribute they can set).

Also fixed formatting according to your directions, thanks.

Patches are attached. If you prefer I can send one big squashed commit. For
review you can look
at them here:
https://github.com/KiCad/kicad-source-mirror/compare/master...qu1ck:plugin-icon

On Thu, Aug 16, 2018 at 11:01 PM Andrew Lutsenko 
wrote:

> 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 
> 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 <
>> anlutse...@gmail.com>:
>>
>>> 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
>>> 
>>>  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  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  = "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.
 

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Clemens Koller]

Hi, Andrew!

On 17/08/2018 07.10, Andrew Lutsenko wrote:
> Or are you asking to update docs in the repo?

I suggested to have your sample pluging and icon bundled with your patch, ready 
for git to apply (once committers agree...).

Clemens

On 17/08/2018 07.10, Andrew Lutsenko wrote:
> 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 
> 
>  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  > 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   = "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@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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

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  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 <
> anlutse...@gmail.com>:
>
>> 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
>> 
>>  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  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  = "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@lists.launchpad.net
>>> > Unsubscribe : https://launchpad.net/~kicad-developers
>>> > More help   : https://help.launchpad.net/ListHelp
>>> >
>>>
>>> ___
>>> Mailing list: https://launchpad.net/~kicad-developers

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrey Kuznetsov]

Please do not throw errors like unable to load icon to users, that is
extremely annoying to users. It is the developer's responsibility to make
sure the icon is loaded correctly, not the users, so users should not be
bothered, since they cannot do anything about it anyway.

On Thu, Aug 16, 2018 at 2:26 AM Andrew Lutsenko 
wrote:

> Ok, tested this further and thought that it's silly that buttons on
> toolbar have icons but menu items have generic hammer.
>
> Second patch adds icons to menu and fixes compatibility with plugins that
> don't define icon.
>
> [image: actionmenu.png]
>
> If icon file doesn't exist or can't be loaded (wrong format for example)
> pcbnew will throw an error message
> and will continue with no button for that plugin and generic hammer icon
> in menu. I think that's reasonable
> behavior since it lets user know that the plugin is somewhat broken, but
> let me know if you want me to
> change it.
>
> Regards,
> Andrew
>
> On Wed, Aug 15, 2018 at 2:31 AM 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 = "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@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
-- 
Remember The Past, Live The Present, Change The Future
Those who look only to the past or the present are certain to miss the
future [JFK]

kandre...@gmail.com
Live Long and Prosper,
Andrey
___
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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Seth Hillbrand]

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 <
anlutse...@gmail.com>:

> 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
> 
>  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  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  = "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@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
>
___
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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

Hi Konstantin,

Thanks for feedback.

I like the idea that plugin writers can opt out of showing the button on
toolbar, I will add this.

Ultimately user should have control over it. If all plugin writers start
adding icons top
toolbar will run out of space. User should be able to choose which plugins
to keep on the
toolbar and which ones should be just in the menus. But that is a separate
problem to solve.

> Also, probles with icon don't must take affect on visibility of toolbar
button. Just use default icon.

I'm not sure about this one. It sounds good for one plugin but if there are
few with icons that failed
to load they will be indistinguishable without hovering with mouse until
tooltip pops up which is far
from ideal.
I would argue that it's best to keep those in menu where you can read the
plugin name right away.

Andrew

On Thu, Aug 16, 2018 at 5:26 AM Константин Барановский <
baranovskiykonstan...@gmail.com> wrote:

> чт, 16 авг. 2018 г. в 12:26, Andrew Lutsenko :
>
>> If icon file doesn't exist or can't be loaded (wrong format for example)
>> pcbnew will throw an error message
>> and will continue with no button for that plugin and generic hammer icon
>> in menu.
>>
>
> Hi Andrew!
>  I think will be better to add the separate option that will controls
> visibility of plugin on toolbar. For example:
>
> import os
> import pcbnew
> import wx
>
> class Plugin1(pcbnew.ActionPlugin):
>
> def defaults(self):
> self.name = "Dummy Plugin 1"
> self.category = "Read PCB"
> self.description = ""
> *self.show_on_toolbar = true*
> self.icon_file_name = os.path.join(os.path.dirname(__file__),
> 'icon.png')
>
> def Run(self):
> wx.MessageBox("Plugin 1")
>
> Plugin1().register()
>
> Also, probles with icon don't must take affect on visibility of toolbar
> button. Just use default icon.
>
> Anyway, good work. Thanks.
>
___
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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

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

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  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  = "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@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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Константин Барановский]

чт, 16 авг. 2018 г. в 12:26, Andrew Lutsenko :

> If icon file doesn't exist or can't be loaded (wrong format for example)
> pcbnew will throw an error message
> and will continue with no button for that plugin and generic hammer icon
> in menu.
>

Hi Andrew!
 I think will be better to add the separate option that will controls
visibility of plugin on toolbar. For example:

import os
import pcbnew
import wx

class Plugin1(pcbnew.ActionPlugin):

def defaults(self):
self.name = "Dummy Plugin 1"
self.category = "Read PCB"
self.description = ""
*self.show_on_toolbar = true*
self.icon_file_name = os.path.join(os.path.dirname(__file__),
'icon.png')

def Run(self):
wx.MessageBox("Plugin 1")

Plugin1().register()

Also, probles with icon don't must take affect on visibility of toolbar
button. Just use default icon.

Anyway, good work. Thanks.
___
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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Clemens Koller]

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  = "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@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

Re: [Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

Ok, tested this further and thought that it's silly that buttons on toolbar
have icons but menu items have generic hammer.

Second patch adds icons to menu and fixes compatibility with plugins that
don't define icon.

[image: actionmenu.png]

If icon file doesn't exist or can't be loaded (wrong format for example)
pcbnew will throw an error message
and will continue with no button for that plugin and generic hammer icon in
menu. I think that's reasonable
behavior since it lets user know that the plugin is somewhat broken, but
let me know if you want me to
change it.

Regards,
Andrew

On Wed, Aug 15, 2018 at 2:31 AM 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 = "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
>


0002-Add-icons-to-action-menu-items-as-well.patch
Description: Binary data
___
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

[Kicad-developers] [PATCH] RFC: toolbar button support for action plugins

[Andrew Lutsenko]

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 = "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


0001-Add-toolbar-buttons-for-action-plugins.patch
Description: Binary data
___
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