← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] libedit: part deletion

 

Awesome! Glad it was something simple. Everything else looks good - I
just pushed it to master. I also pushed a commit to remove unit display
from this dialog entirely, since there's no support for deleting
individual units. Apologies for invalidating the work you did to sniff
out this bug - as it seemed intermittent I wasn't 100% sure it *was*
unit-related :\


On Sun, Feb 05, 2017 at 03:21:18PM +1100, 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>
> 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.
> > > > >
> > > >
> >
> >



References