Hi Wojciech, On Tue, 13 Nov 2018, Wojciech wrote: > Hi! > > I sent this message yesterday, but to a wrong address (it changed probably?). > > Here's the recipe to reproduce the bug (this is for board attributes): > > 1. Choose in menu: Edit -> Edit properties of -> Layout > 2. Select an attribute (a/PCB::grid::unit) > 3. Click Remove attribute - nothing happens > > Removing attributes of subcircuits on board actually works, but not with ones > added manually. Here's the recipe: > > 1. Place a subcircuit (e.g. 1206) > 2. Select it and press {e p} > 3. Click Add attribute, write something, ok > 4. Select new added attribute and click Remove attribute - looks good so far, > but: > 5. Close properties dialog > 6. Open again ({e p}) - the added attribute is still there and cannot be > removed > Thanks, I managed to reproduce it. Unfortunately it led to a cluster of bugs that can not be fixed easily. Some needs to be done in a cleanup phase, so this will have to wait until the next cycle. Problems: 1. the property editor works only on selection, not on a single object; in our case we'd want to edit the subc only, but we first have to select it because the property editor doesn't have an "object" mode only a "selected" mode. The current GUI implementation is gtk-only and we have a custom API between the gtk GUI and the propedit plugin. It was invented long ago, before DAD. So I thought each gui-affecting feature would grow such an API then each HID would reimplement the GUI dialog and use that API. But that would have lead to an O(n*m) situation in development time (n=number of features, m=number of HIDs). So we now have DAD instead, which means only O(n+m). So the property editor UI part, both the GUI and the action, will be rewritten from scratch, and then it will follow the object/selected/#id mechanism which I want to make standard for most similar actions. This matters, because an important feature of propedit is that it can edit multiple objects at once, so it needs to work on selection as well. Which leads to: 2. subc selection mechnism is sort of broken. We inherted this idea from mainline elements: when you select an subc, it also selects all subc parts. Normally this is good, this why you get all objects of the subc highlighted. But in our specific case this also means you are not adding the attribute to the subc only, but to all parts as well. One solution will obviosuly be that propedit will work on object instead of selection too, but this won't solve the case when you want to select multiple subcircuits to unify some of their properties or attributes. So I will need to think over how we can separate subc selection from subc part selection without making the system too complicated for users. Whatever I come up with will be a big change, should happen in a cleanup phase. 3. a smallish bug: add and remove are not symmetric: even with the above bug, if add attribute adds it to all subc parts as well, remove should remove from all too - so until you look at a subc part separately, you wouldn't even notice bug #2. However, remove seems to remove it only from the subc, but nothing else. That's why you still see it in the dialog: after removing it from the subc, you still have it on the subc parts because of bug #2 and the asymmetry bug. Thanks for reporting this. I am going to save this mail in svn and add a few TODO entries. ----- Possible solution to #2: - keep the selection as is (for the highlight): deep - provide an alternative, shallow selection mechanism - make sure query() uses shallow selectuin - provide an action and a GUI for selection stats: how many different type of objects are selected, remove selection of specific types - the propedit dialog should offer the same stat and invoke the selection stat gui if the user wants to remove some selections Add a subc context menu for shallow select, make sure second select (unselect) works properly in the shallow case.