gtk4-rs icon indicating copy to clipboard operation
gtk4-rs copied to clipboard

Don't implement `Copy` on certain BoxedInline structs

Open SeaDve opened this issue 1 year ago • 5 comments

Currently, it implements Copy, which could cause unexpected behaviors. I think it is better to just implement Clone, so the user can explicitly create a copy of the iterator.

This is also the reason why Range from std lib doesn't implement Copy, even though the underlying type implements copy.

Example:

let mut text_start = text_end;
text_start.backward_line();

Does text_end also move backward a line? (Spoiler: it does not, since it is creating a copy behind the scenes)

let mut text_start = text_end.clone();
text_start.backward_line();

Here, it is explicit that we are making a deep copy of the text_end.

SeaDve avatar Nov 01 '23 01:11 SeaDve

That makes sense, yes. We should check other types that currently implement Copy too, here and in gtk-rs-core.

Can you create an MR and go over these?

sdroege avatar Nov 01 '23 07:11 sdroege

A quick look on gtk4-rs which has possibly problematic Copy are the following:

  • GtkTreeIter (Though doesn't seem to have any methods nor impls)
  • GtkTextIter
  • GskRoundedRect (has &mut methods)
  • GtkBorder (has &mut methods)
  • GtkTopLevelSize (has &mut methods)

In gtk-rs-core, there are no iterators, but there are these:

  • PangoGlyphGeometry (has &mut methods)
  • PangoGlyphInfo (has &mut methods)
  • GrapheneMatrix (has &mut methods)
  • GDate (has &mut methods)
  • GrapheneRect (has &mut methods)
  • PangoRect (has &mut methods)
  • PangoMatrix (has &mut methods)
  • GStringBuilder (BoxedInline, but somehow no Copy impl?)

SeaDve avatar Nov 02 '23 06:11 SeaDve

GStringBuilder (BoxedInline, but somehow no Copy impl?)

Only ones that have no copy/clear function are Copy IIRC. If you need to free memory or do more things than a memcpy then it's not Copy and can't be.

The ones you listed seem all OK (gtk-rs-core). For gtk4-rs I'll let @bilelmoussaoui comment.

sdroege avatar Nov 02 '23 06:11 sdroege

GskRoundedRect (has &mut methods)

Not sure how to handle this one

GtkBorder (has &mut methods)

But the type also provides a copy function? gtk_border_copy, where that is supposed to be used in this case?

GtkTopLevelSize (has &mut methods)

This was fixed already

bilelmoussaoui avatar Jan 20 '24 20:01 bilelmoussaoui

For TextIter, see also #1280

bilelmoussaoui avatar Jan 20 '24 20:01 bilelmoussaoui