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 <[email protected] > <mailto:[email protected]>> 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 > <[email protected] <mailto:[email protected]>> > > 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 : [email protected] > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
