← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] libedit: part deletion

 

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>
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
> [2] 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>
> > 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.
> > > >
> > >
>
>

Attachment: part_deleter_3.patch
Description: Binary data


Follow ups

References