kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #27608
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