PD: Rename {Sub}ShapeBinder::Support properties - fixes #7052

[jcoalson]

⚠️ Superseded by #12714 ⚠️ If that PR is merged, this can be closed.

PartDesign: Rename ShapeBinder::Support and SubShapeBinder::Support properties to ObjectSupport, to avoid name conflict with Part::AttachExtension::Support.

Fixes #7052

This bug is triggered when using the attach extension on a binder.

Hopefully the comments on handleChangedPropertyName() are self-explanatory, re: how legacy files are handled.

[jcoalson]

PS I'm not a big fan of the word Support in this property name given the confusion it causes, but I don't know the project well enough to come up with an alternative. Now would be a good time to pick something better if needed though.

[adrianinsaval]

I agree support is a bit confusing, @realthunder could you give some insight on what exactly this is supposed to represent? Seems to me like "Source" could be a more fitting name.

Suggestions welcome on what this should be named

[jcoalson]

Preferably something unique enough to not cause conflict in the future 😁

[adrianinsaval]

suggestions from chrisb on the forum:

Shape (according to Part booleans)
Base (because it tastes a bit like a BaseFeature)
Profile (according to PartDesign features)
Originals (according to PartDesign patterns)
Source (I'm very sure it is used somewhere)

That makes two people voting for Source 😉, Shape is a property that already exists so can't use it. Profile doesn't seem right to me. Any of the others is ok with me.

[realthunder]

I suggest to change Support property of the AttachmentExtension to AttachmentSupport, similar to AttachmentPlacement This is what I did it in my branch. Extension is meant to alter an object, so by right it should be careful to not causing any conflict. There may be other objects having a property named Support.

[jcoalson]

It's funny, I took that exact same approach first (right down to the same name). But it required changing quite a bit more code and seemed like it would end up causing more migration headache given that migrated projects can't be used on old versions.

I think the core problem is that when Document.xml is written, the scoping of properties is lost. The current code seems to works fine (properties stay members of their own objects in both C++ and Python land) right up to the point where you save and then reopen the project.

[jcoalson]

PS I'd be happy to defer to a PR from you with the AttachmentSupport change. I just want the pain to go away. :) I stopped that tack midway after finding uses of Support in inline python code strings and I was doubtful of being able to correctly identify all the places that would not be checked at compile time.

However this PR doesn't preclude also fixing AttachExtension and it's not too disruptive.

[jcoalson]

Ugh, there seem to be assumptions in the code that a ShapeBinder is special kind of feature that can sometimes belong to a class of 'supported-by-sketch'-like features, all of which are assumed to have a Support property.

E.g. after this patch, if I highlight a sketch in the tree view and create a shape binder, I get an exception 'Part.Feature' object has no attribute 'Support'. The offending line is here in PartDesignGui::fixSketchSupport()

(What is supposed to happen is the task window opens up with the Object (text) field pre-filled with the sketch name.)

This seems to be another reason to make the fix in AttachExtension.

[adrianinsaval]

@wwmayer I would appreciate also having your opinion on which property should be renamed here

[wwmayer]

I suggest to follow RT's fix and change it in the AttachmentExtension

[adrianinsaval]

for reference:
realthunder@0191f4e

however there's likely more to it than that

[jcoalson]

@realthunder, trying to understand your commit... can I ask why you did not also update the property name on this line? https://github.com/realthunder/FreeCAD/blob/2022.06.25-edge/src/Mod/PartDesign/Gui/Command.cpp#L252

[realthunder]

Likely I missed that bit. But since Datum does not have Support property, it still works. I kept Support property in Attacher for backward compatibility reason, and synchronize the content of Support and AttachmentSupport, so existing working code will not break. And it will also work when opening in older version FreeCAD.

[jcoalson]

^ Superseded by #12714

[chennes]

Closing in favor of #12714