← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Fix for bug/1754049

 

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.

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>:

> 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>:
>
>> 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
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
>
>

Follow ups

References