← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] libedit: part deletion

 

I committed this patch.  Thanks Oliver.  Please keep in mind when
submitting patches that fix bugs to include the bug report information
in the commit message.  At a minimum, "Fixes: lp:######" is required.
It's also not a bad idea to include the link to the bug report even
though our commit hook should take care of this automatically.

Cheers,

Wayne

On 2/4/2017 11:21 PM, Oliver Walters wrote:
> Ok, I have found the cause of this behaviour:
> 
> As the selection dialog's true purpose is to *select* a component, it
> was setting the m_unit* variable to the selected unit. If the currently
> selected part did not have at last as many units, it was showing a
> non-existent view of the selected part.
> 
> This has been addressed in the attached version of the patch.
> 
> Hopefully no more errors!
> 
> On Sun, Feb 5, 2017 at 12:36 AM, Chris Pavlina <pavlina.chris@xxxxxxxxx
> <mailto:pavlina.chris@xxxxxxxxx>> wrote:
> 
>     Nope, this doesn't fix the weird behavior. However, I have noticed that
>     it only happens in some specific case (the conditions of which I have
>     yet to properly identify), I just got lucky and found that case first.
>     Here are steps that should reproduce this:
> 
>     1) Create and load a new project.
>     2) Open eeschema. Remove all libs, then add the library at [1], [2].
>     3) Open libedit and choose the Atmel library I attached.
>     4) Load component ATMEGA168A-AU.
>     5) Delete unit B of ATSAM4E16CA-AU.
> 
>     Follow those steps *exactly* and it should happen.
> 
>     Also, I'm not really happy with how units are being handled here. If I
>     select a single unit, it shouldn't delete the whole part - that's
>     misleading. If it's too troublesome to implement single-unit delete, we
>     should add a mode to the component selector widget to disable display of
>     units.
> 
>     **However**, even if we just drop the unit selection from the dialog, I
>     really think it might be worth looking into why this happens when using
>     it...this is weird, may indicate some deeper bug...
> 
>     [1] https://misc.c4757p.com/Atmel.lib
>     <https://misc.c4757p.com/Atmel.lib>
>     [2] https://misc.c4757p.com/Atmel.dcm
>     <https://misc.c4757p.com/Atmel.dcm>
> 
>     On Sat, Feb 04, 2017 at 07:14:42PM +1100, Oliver Walters wrote:
>     > Chris,
>     >
>     > I have attached an updated patch that (at least on my end) fixes the
>     > unit-deletion bug.
>     >
>     > Can you please confirm that you no longer see the erroneous behaviour?
>     >
>     > I have also included your preselect patch.
>     >
>     > Cheers, Oliver
>     >
>     > On Thu, Feb 2, 2017 at 10:33 PM, Chris Pavlina
>     <pavlina.chris@xxxxxxxxx <mailto:pavlina.chris@xxxxxxxxx>>
>     > wrote:
>     >
>     > > On second thought, I'm reverting it, I found a weird bug.
>     Apologies for
>     > > the accidental push, I'll be more careful next time.
>     > >
>     > > I *was* investigating what I thought was a minor bug (i.e.
>     didn't keep
>     > > me from pushing the patch), but I found this. To reproduce:
>     > >
>     > > 1) Select a single-unit part
>     > > 2) Delete one unit of a different, multi-unit part
>     > >
>     > > The current part gets messed up, like: https://misc.c4757p.com/
>     > > messed-up-part.png
>     > >
>     > > Not sure if that's a rendering issue or if the pins are actually
>     gone.
>     > >
>     > > What I *was* looking at is that I thought deleting a unit would just
>     > > delete the entire part, and I was going to suggest one of two
>     possible
>     > > fixes:
>     > >
>     > > 1) Modify the component selector to have a mode where it doesn't
>     delete
>     > > units
>     > > 2) Actually delete the unit, adding code to filter the part and
>     remove
>     > > anything on the unit to be deleted and reorder subsequent units.
>     > >
>     > > I was going to offer to write #2 myself, because that'd be
>     really useful
>     > > to me. But you should probably fix the bug described above first and
>     > > resubmit.
>     > >
>     > > ALSO, I was going to contribute a small patch to this to
>     preselect the
>     > > current component just like I added to Load Component recently - I
>     > > attached a diff for that in case you want it, I had it all done
>     before
>     > > noticing this.
>     > >
>     > > On Thu, Feb 02, 2017 at 06:15:57AM -0500, Chris Pavlina wrote:
>     > > > Oops, I didn't mean to push this one at the same time, clumsy push
>     > > > command - but it looks good too. I always hated that delete
>     dialog.
>     > > > Thank you!
>     > > >
>     > > > On Thu, Feb 02, 2017 at 09:42:01PM +1100, Oliver Walters wrote:
>     > > > > The attached patch removes the clunky "Delete Single Part"
>     dialog in
>     > > > > libedit, and re-uses the existing part-selector to find a
>     component to
>     > > > > delete.
>     > > > >
>     > > > > Now, the user can use the filter tool to select a component for
>     > > deletion.
>     > > >
>     > >
> 
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 


Follow ups

References