kicad-developers team mailing list archive

[hauptmech <hauptmech@xxxxxxxxx>]

On 20/03/18 11:19, Seth Hillbrand wrote:
> Hi hauptmech-
>
> I disagree with your assessment of whether we should allow the 
> creation of broken boards.  I don't think that we should ever allow 
> KiCad to create a board that it cannot modify itself.  Doing this is 
> to invite confusion and bug reports.

Seth I'd rather you say that you think this puts the board in a broken 
state and you don't think we should do it. I'm ok  with that. But when 
you summarize my assessment using your own framing and then disagree 
with it, it makes it hard to work on this objectively.

I don't think leaving things as they are will result in broken boards. 
The boards will load and save cleanly until the GUI can catch up.

I think the best option of what you suggested is to allow selecting 
modules via the reference point. This allows selecting (via box select I 
presume) modules that have nothing showing on any visible layers. 
However, this is beyond what I am able to do for this patch.

I am able to implement either of the other two options suggested, 
deleting the module or preventing deletion of the layer. The layers are 
being deleted because of an issue with the DRC according to code 
comments, so preventing deletion may not have the affect that is desired.

I already said why I don't think deleting the modules are the way to go, 
however I am happy to implement whatever you guys want.

Were this work for version 6 I'd say that the kicad team really needs to 
rethink the setup layers dialog and how it fits into the workflow.

Anyway, let me know. I just got a pile of new work and would like to put 
this to rest.

> I don't have an opinion as to which is preferable, deleting all of the 
> module or preventing deletion of a selectable layer.  But either one 
> will allow KiCad to edit the file later.  Alternatively, we might 
> consider allowing KiCad to delete/edit a module using only the 
> remaining reference point.
>
> As it is, the patch resolves an issue and creates another.  I'd prefer 
> we didn't kick the can down the road like this.
>
> -S
>
>
>
> 2018-03-18 2:09 GMT-07:00 hauptmech <hauptmech@xxxxxxxxx 
> <mailto:hauptmech@xxxxxxxxx>>:
>
>     Updated patch attached. Coding standard stuff fixed.
>
>     I took a second look at the case where all items in a module are
>     deleted.
>
>     I checked the parser and writer. In the case where all items are
>     deleted, the modules are still OK as far as reading and writing to
>     the file.
>
>     I don't think deleting the module is the right way to go. They are
>     on layers which cannot be deleted in the current implementation.
>
>     Most modules will have pads on the undeleteable layers, so they
>     will be fine.
>
>     For the case of someone with modules composed only of
>     non-top/bottom elements and who are messing with the layer setup
>     midway through the design, I think we should just leave it as it
>     is. Either they are a ninja doing things that kicad should stay
>     out of the way of  or they are a very confused beginner for whom a
>     little extra cruft in the file is the least of their problems.
>
>     -hauptmech
>
>
>
>
>     On 15/03/18 05:17, Seth Hillbrand wrote:
> >     Hi hautmech-
> >
> >     Looks good.  Two comments:
> >
> >     - Code style.  There are a couple of small issues: one line is
> >     too long, there should be spaces in parentheses for
> >     pad->GetParent()->Remove(pad) and no space before parentheses in
> >     "if ( pad->GetLayerSet()..."
> >
> >     - We need to handle the case where all items in a module are
> >     deleted.  Currently, the code will remove all items except the
> >     value and reference.  This means that we can no longer select the
> >     module in pcbnew to delete it. Either we need to fully delete the
> >     module when the last selectable item is removed or we need to
> >     keep sufficient information to allow the module to be selected
> >     when the layers are re-enabled.
> >
> >     Best-
> >     Seth
> >
> >     2018-03-14 2:54 GMT-07:00 hauptmech <hauptmech@xxxxxxxxx
> >     <mailto:hauptmech@xxxxxxxxx>>:
> >
> >         https://bugs.launchpad.net/kicad/+bug/1754049
> >         <https://bugs.launchpad.net/kicad/+bug/1754049>
> >
> >         This patch follows the edict (stated in
> >         dialog_layers_setup.cpp) that items on a layer that is
> >         removed must be deleted.
> >
> >
> >
> >         _______________________________________________
> >         Mailing list: https://launchpad.net/~kicad-developers
> >         <https://launchpad.net/%7Ekicad-developers>
> >         Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >         Unsubscribe : https://launchpad.net/~kicad-developers
> >         <https://launchpad.net/%7Ekicad-developers>
> >         More help   : https://help.launchpad.net/ListHelp
> >         <https://help.launchpad.net/ListHelp>