Part: Rename AttachExtension::Support property - fixes #7052

[jcoalson]

⚠️ Supersedes #12579

Summary

Part: Rename AttachExtension::Support property to AttachmentSupport, to avoid name conflict with base features, e.g. ShapeBinder.

Fixes #7052

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

Details

When projects are saved, the scoping of object properties is lost, which means a class must be careful not to use the same name for a property as another class if the two classes can be composed via inheritance or extension into a single feature.

AttachExtension and ShapeBinder currently conflict on Support. #12579 renames ShapeBinder's but in discussion it was noted that since AttachExtension is an App::Extension, it should be the one to avoid conflicts. This PR is longer due to the ubiquity of attachments, and trickier due to the lack of compile-type checking on the python side, but I think it's worth it.

The approach taken here is to do a full migration instead of keeping a deprecated Support property around, which could get very confusing since attachments are so widely used.

While I was there, I moved the migration for an old type change out of Part2DObject into AttachExtension where it belongs, which cleaned up some cruft.

[jcoalson]

PS I didn't update any of the numerous docstrings, I assume that happens on crowdin?

[adrianinsaval]

PS I didn't update any of the numerous docstrings, I assume that happens on crowdin?

are you talking about documentation in cpp/py files or in .ts files? The later is managed with crowdin but code documentation should be updated here.

[jcoalson]

I meant the .ts files. I updated everywhere else.

[adrianinsaval]

Let's ship it and keep an eye out for complaints to fix

[Zolko-123]

Rename AttachExtension::Support property to AttachmentSupport,

Are you MAD ? Do you know how many things depend on this ?

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

Who uses an attachment with a ShapeBinder ???

[jcoalson]

Rename AttachExtension::Support property to AttachmentSupport,

Are you MAD ? Do you know how many things depend on this ?

Nothing breaks, old projects are automatically migrated when loaded. edit: I guess you mean 3rd party code.

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

Who uses an attachment with a ShapeBinder ???

How would you solve this problem without it?

[Zolko-123]

YOUR solution to YOUR problem is to destroy every-body elses work ?

[adrianinsaval]

THE solution to A problem... and it's fairly easy to adapt your code. I think something like this should work:
0001-Port-to-FreeCAD-0.22.0.36274.patch.txt

[Zolko-123]

How many people did have this "problem" that you talk about ? Where was it ever reported ?

[jcoalson]

There are multiple forum threads and at least one github issue, linked right here in the PR comments.

This is your third comment about things discussed for over a year and accessible through this very same page. I'm not sure why you don't want to read about it first before commenting.

[snelweg]

Wow, this is so stupid and will make FC even more unloved.
How can you ruin someone's work like a dictator? Fix the unbelievable still existing topo error if you want to fix a REAL problem. Did the guys at Ondsel make you do this?
F*ck FC, moving on to OnShape.

[jcoalson]

i still love you freecad

[3x380V]

@jcoalson, ordinary love needs some proof occasionally to not perish, so let me introduce a challenge in file. In was created using 0.21.2:

As you can see, all datums have Support assigned. However opening the very same file in current version leaves AttachmentSupport empty:

which breaks all the models using datums and local coordinate systems and hopefully explains all that angry comments you read above. Also note @FlachyJoe's statement in #12894 (comment)

[FlachyJoe]

in current version

Which one? See #14429 too.

[3x380V]

Top most commit is 26714820cdfe ("Fix compiler warnings"), #14429 is not related to this problem although it is included in tested version.

[3x380V]

Which one? See #14429 too.

It is #13964, which brings code unaware of property rename. Also extHandleChangedPropertyName was renamed meantime. I will create separate issue for that.