FreeCAD Forum

Please test and review :
https://github.com/FreeCAD/FreeCAD/pull/5480
And if someone can merge

This is certainly a cool feature, thank you!
Regarding the code I have a few comments and suggestions, though.

The first thing that caught my eye: there are no unit tests. I think new features should be covered by automated tests right from the beginning (Ideally you start with the tests: no failing tests means nothing to do ). IIRC @chennes recently added tests for Copy/Paste in the spread sheet module, so there should be some code to look at already.

Then I suggest to break the long methods into smaller, dedicated methods. A single method with 120 lines and 8-9 levels of indentations are too much for my taste. You already wrote comments over each "section". Those give nice method names. Overall this makes the code easier to read (also for your future self).

You also put FreeCAD's XML fragment as "text/plain" into the clipboard. Followed by some string magic to check if it is that XML when reading it back. I suggest to use a dedicated media type instead and check for that directly when pasting.
Does anyone know whether we have a media type for FreeCAD's xml format already defined somewhere? Something like "application/vnd.freecad.fcstd-document+xml" or here specifically "application/vnd.freecad.fcstd-document-fragment+xml"?
(One could even think of adding the same data in different formats, for different purpose. i.e. one for pasting between/within FreeCAD, one as "image/svg+xml" for direct pasting in inkscape, a human readable version as "test/plain", ...)

jonasb wrote: ↑Wed Feb 02, 2022 9:14 pm This is certainly a cool feature, thank you!
Regarding the code I have a few comments and suggestions, though.

Thanks for your feedback !

Regarding unit tests I have no clue what they are and how they work. I'm not sure I can handle another layer of complexity tbh. I'm already struggling all the way to understand the code structure. Having to do that again for something non-vital makes me want to stop.

Personally I prefer standing function. But I can see reason that it's a bad habit and that smaller functions enable more code reusability and probably better readability. So I should try to split it.

This sounds like a good idea to have data in different formats to be compatible with inkscape and others. It would be awesome. Though I'm not sure to be up to the task right now as I have a few other features that need to be finished.

Let's continue the discussion in github directly at your PR. That's better than having it scattered over different places.

@0penBrain already pointed you to the standard copy/paste functions. The mime type thingy I mentioned above is already used there.
And just for the record, directly above MainWindow::createMimeDataFromSelection() the currently used mime types for document objects in the clipboard are defined: https://github.com/FreeCAD/FreeCAD/blob ... .cpp#L1640