sugarlabs / sugar

Sugar GTK shell
GNU General Public License v3.0
252 stars 240 forks source link

Add drag from clipboard to journal to store data #960

Closed chimosky closed 1 year ago

chimosky commented 1 year ago

@quozl @srevinsaju kindly test and review.

Will fix object recopying to the clipboard after it's dragged and saved to the journal in another PR.

It seems to have some weird conditions with drags from browse. If I drag an image from browse to the journal, it fails [1]. But if I go browse->clipboard then clipboard->browse it works.

This is still the case and I don't think it should change.

Seeing as this is a feature, would one commit be preferred?

quozl commented 1 year ago

Minor review comments.

Perhaps _ContextMap from cliboardtray should not have underscore prefix now that it is used elsewhere.

I'm surprised GLib.strfreev is needed, as usually the Python API does all the memory management when objects are deleted. And then to use "data" shortly after it is freed doesn't make sense to me.

chimosky commented 1 year ago

Minor review comments. Perhaps _ContextMap from cliboardtray should not have underscore prefix now that it is used elsewhere.

Agreed.

I'm surprised GLib.strfreev is needed, as usually the Python API does all the memory management when objects are deleted. And then to use "data" shortly after it is freed doesn't make sense to me.

Yeah agreed, I'll change it. I'd added it as a measure as I didn't know if memory management for objects were done automatically, it was also something that wasn't used elsewhere in the API so it was difficult to add it. I'll remove it. Thanks!

chimosky commented 1 year ago

Changes made, @quozl kindly review.

quozl commented 1 year ago

Looks fine.