lol, I was just going to ask you why that line is even there if _no_ components can have a zero unit, and then decided to git-blame it and realized _I_ wrote that line.
I think the reason I did that is that the new selector returns zero when the user has selected a root component - it only returns a nonzero unit if a unit was actually selected. The documentation for the selector points this out, but clearly it's sufficiently confusing that I got it wrong in my own code :\ Probably best to fix DIALOG_CHOOSE_COMPONENT::GetSelectedAlias() to always return a valid unit. I wouldn't go so far as to make all CMP_TREE_NODE_ALIASes have Unit=1; that would have the desired effect, but that class is meant to represent just an alias and an alias by itself doesn't really have a unit number. On Thu, Mar 30, 2017 at 03:26:11PM -0400, Wayne Stambaugh wrote: > The problem is line 169 of eeschema/getpart.cpp. The component unit > cannot be 0 even for components that do not have multiple units. There > probably should be an assertion in the SCH_COMPONENT ctor to warn devs > about this. > > On 3/30/2017 3:24 PM, Chris Pavlina wrote: > > Ooooops. Sorry. > > > > I caught one place where I had done that. Thought that was all. :\ > > > > > > On Thu, Mar 30, 2017 at 03:13:31PM -0400, Wayne Stambaugh wrote: > >> I believe since the new component chooser dialog has been implement > >> which hasn't been very long. > >> > >> On 3/30/2017 2:53 PM, Maciej Suminski wrote: > >>> Do you know for how long the bug was present? It could help estimate the > >>> number of schematic files affected. > >>> > >>> As much as I hate windows popping up, I do not think notifying the users > >>> would be that annoying. It is going to happen only once for each invalid > >>> file. I doubt there are people working on 10+ designs in parallel. > >>> > >>> I vote for #3. IMHO #4 is acceptable too. I would avoid #1 and #2 in > >>> particular. > >>> > >>> Regards, > >>> Orson > >>> > >>> On 03/30/2017 07:33 PM, Wayne Stambaugh wrote: > >>>> I have found and fixed the bug in this bug report: > >>>> > >>>> https://bugs.launchpad.net/kicad/+bug/1677282 > >>>> > >>>> This bug creates invalid schematic files which sets the component unit > >>>> flag to 0 and breaks the netlist generator. The fix itself is simple. > >>>> What is not simple is what to do about the invalid schematic files that > >>>> have already been created. I added code to schematic parser to fix this > >>>> but this creates a dilemma. Technically the schematic is modified which > >>>> begs the question, what to do next. None of the following choices are > >>>> particularly appealing: > >>>> > >>>> 1) Do nothing and leave the file in an invalid status until the next > >>>> time the user saves the schematic. This is probably the most convenient > >>>> but what about broken schematics being used by versions of kicad prior > >>>> to this fix? > >>>> > >>>> 2) Set the schematic modified status which will trigger a save warning > >>>> when eeschema is closed even if the user hasn't made any changes. We > >>>> already do this with SCH_SCREEN::SchematicCleanUp() that gets called > >>>> lots of places outside of schematic editing. The problem with this is > >>>> that the user has no idea why they are getting a save warning when they > >>>> didn't actually change anything. > >>>> > >>>> 3) Set the schematic modified status and inform the user that there was > >>>> an error in their schematic file that was repaired on load and requires > >>>> saving. This is the most informative for the user but reeks of nagware. > >>>> > >>>> 4) Silently save the corrected schematic with no user interaction. This > >>>> will undoubtedly make VCS users unhappy. > >>>> > >>>> As much as I hate nagware, I like unexpected save warnings when I > >>>> haven't changed anything even less so I'm leaning towards option 3. Any > >>>> feedback would be appreciated. > >>>> > >>>> Thanks, > >>>> > >>>> Wayne > >>>> > >>>> _______________________________________________ > >>>> 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 > >>> > >> > >> _______________________________________________ > >> 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
