Symbol Library Editor bug: editing a library by deleting a part in it, or by reverting after copying a part into it, minimizes the library (un-expands) it uncommanded

[Gabriel Staples]

Application: kicad
Version: 6.0.0-rc1-unknown-a5c892d~84~ubuntu14.04.1, release build
Libraries:
    wxWidgets 3.0.2
    libcurl/7.35.0 OpenSSL/1.0.1f zlib/1.2.8 libidn/1.28 librtmp/2.3
Platform: Linux 4.4.0-112-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.54.0
    OpenCASCADE Community Edition: 6.8.0
    Curl: 7.35.0
    Compiler: GCC 4.8.4 with C++ ABI 1002

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

------------------------

In the Symbol Editor, every time I right-click in the left-hand menu to delete a part from a library, or if I copy and paste a new part into the library, then right-click and say "revert", it minimizes the library. This shouldn't happen. It should retain the information about whether or not I have the library expanded or minimized, and not minimize (un-expand) it every time I remove a part from it. See attached screenshot.

[Jeff Young (jeyjey)]

I can't reproduce this on OSX. If someone on GTK wants to look into it, the routines that are /supposed/ to handle this are LIB_TREE::getState() and LIB_TREE::setState().

FWIW, trying to reproduce this did uncover 3 other bugs, one of them a crasher:
1: when reverting a library the confirmation dialog has the empty string for the library name
2: when reverting a library we assert when trying to clear the screen
3: when reverting a library with no symbols we crash trying to see if the current symbol needs reloading

I've merged fixes for all of these:
caa38a61939d3c1babd4174c7e1ec62e961adcb2
61b9412c811fbbe3317274806b5a7cca0990da26
7ab5bd2339df699138789454f544412f611c8265

[Gabriel Staples (ercaguy)]

In my attempt to reproduce the bug, I found some more bugs too (some of the ones you found I'm sure?), and didn't (yet) reproduce the original bug. Here's a video.

00:05: kicad warning about grid size
00:28: I reverted the eRCaGuy_Misc library, and it should have deleted the BSS138_copy part, but it didn't.
00:50: I repeatedly click on the BSS138_copy part and it won't open (probably because I reverted it?)
01:02: I revert the 2nd copy, but again, it didn't delete it.
01:16: I deleted BSS138_copy and it crashed the program.

[Gabriel Staples (ercaguy)]

Ok, I was able to recreate the bug. Here's the video. You have to open the Symbol Editor from Eeschema.

Here's the new video:

00:12: kicad warning about grid size again. Bug?
00:30: I deleted the symbol and it minimized the eRCaGuy_Misc library, but it shouldn't have! <---that's the original bug this ticket was meant to report!
00:52: I reverted and it crashed the whole KiCad program.

[Wayne Stambaugh (stambaughw)]

@Jeff, do any of these bugs affect the 5.0 branch? If so, they should be cherry-picked. The crash bug fix at a minimum.

[Jeff Young (jeyjey)]

The empty-string issue is part of the GUI re-write; the other two are part of the GAL re-write (so none of them are in 5.0.)

[Jeff Young (jeyjey)]

@Gabriel, neither FireFox nor Quicktime will show your videos on my machine....

[Seth Hillbrand (sethh)]

@Jeff- You might need vlc https://www.videolan.org/vlc/download-macosx.html

[Gabriel Staples (ercaguy)]

@Jeff, yes, use VLC. I did the screen recording with OBS studio (https://obsproject.com/) using its default save settings I believe, and I use VLC media player for playing them.

[Jeff Young (jeyjey)]

I still can't reproduce any of the tree-item-collapse behaviours, but I do have steps to reproduce the "orphaned" item in the tree:

Copy a symbol.
Paste it into another library.
Paste it again.
Click on the second copy and revert.
Delete the first copy.
The second copy is now orphaned.

[Jeff Young (jeyjey)]

It was a bit of an insidious bug (with an internal buffer datastructure having the wrong library id in it), so it's possible (though unlikely) that fixing it will clear up some of the other issues.

Anyway, the "fixes" script is the easiest way to get the changelist attached to the bug report so I'll go ahead and use it.

Please re-open with specific steps for what remains.

[KiCad Janitor (kicad-janitor)]

Fixed in revision 023a445e9fcdc6f57759e9e4609502dfc69b61f3
https://git.launchpad.net/kicad/patch/?id=023a445e9fcdc6f57759e9e4609502dfc69b61f3

[Gabriel Staples (ercaguy)]

If I install the latest nightly version will it contain this fix, or how can I test it?

[Jeff Young (jeyjey)]

The next nightly will contain it. It looks like the last one was built about 11 hours ago, so the next should be built in about 13 hours, give or take.

[Gabriel Staples (ercaguy)]

Thanks. I also discovered that the commit hash is contained in the version number:

Ex: Version: 6.0.0-rc1-unknown-82f657e~84~ubuntu14.04.1, release build

The commit hash is 82f657e in this case. So, by pulling the source code from the launchpad git repo (`git clone https://git.launchpad.net/kicad `; see: https://code.launchpad.net/kicad), and searching the git log, I can find when my version was compiled, and I can see where it sits relative to other commits, such as a23a445 above which implements this fix. Now, by installing the latest nightly and looking at the version number, I'm able to see that it does indeed come *after* your commit above, which means it contains this fix. Great! Now I can test it. I recommend you put a summary of what I just said somewhere for others to learn from--perhaps on this page? http://kicad-pcb.org/download/source/

I originally wasted a bunch of time googling about how to figure out when a PPA was last updated, then I discovered the hash thing in the version number.

[Gabriel Staples (ercaguy)]

@Jeff, thanks for the big fixes! Here's a video showing the final thing left to fix as far as I can tell. See attached. Play it with VLC (https://www.videolan.org/vlc/index.html), the world's best open source player for the world's best open source PCB layout software.

[Gabriel Staples (ercaguy)]

I don't know the proper way to indicate this bug is re-opened, so I changed the status from Fix Commited --> New.

[Wayne Stambaugh (stambaughw)]

@Gabriel, if the bug in the bug report was fixed then you should open a new bug report rather than reopen a bug report that has already be fixed. Bug reports are designed to handle a single bug only. There is no way to close part of a bug report so unless multiple bugs are caused by the same fix, they should be reported as separate bugs.

[Gabriel Staples (ercaguy)]

@Wayne, thanks for the guidance. The bug described in the title of this bug report isn't fixed yet, that's why I re-opened it. Should I open a new report or leave this re-opened? I'm fine with either.

I know though that in the process of identifying the bug's behavior we found a bunch of other, bigger bugs (like crashing) which have been fixed by Jeff, though.

[Jeff Young (jeyjey)]

Normally I like to have a new bug opened, but this particular one has come full-circle back to the original report, so it's a bit of an odd case.

In any event, it's now fixed, so no need for a new report. ;)

[Gabriel Staples (ercaguy)]

Awesome, I'll leave it to you to change the status back to fix committed once you've done so. Thanks!

[Jeff Young (jeyjey)]

The Launchpad bug janitor should do that for us (along with a comment including a link to the commit), but it's a bit hit-and-miss.

[KiCad Janitor (kicad-janitor)]

Fixed in revision d2756bedf1b57638d4d742159ac92096ca36db83
https://git.launchpad.net/kicad/patch/?id=d2756bedf1b57638d4d742159ac92096ca36db83

[Wayne Stambaugh (stambaughw)]

@Gabriel, you got right. If the original bug was not fixed by the commit, then setting the status back to new was the correct call. Thanks for the quality bug report. It makes life much easier for developers.