cursive icon indicating copy to clipboard operation
cursive copied to clipboard

Add ability to replace an item in a select view

Open squaresurf opened this issue 7 months ago • 3 comments

This is a more efficient way to replace an item than calling remove_item, then insert_item

squaresurf avatar Dec 04 '23 22:12 squaresurf

Hi, and thanks for the PR!

Could SelectView::get_item_mut() solve your use-case? Or SelectView::iter_mut if you need to access several items. In particular, you should be able to swap two items efficiently this way.

I'm fine adding this method, if only for the S: Into<StyledString> convenience factor. Wondering if it should be set_item vs replace_item, also, we could return the old value? :shrug:

gyscos avatar Dec 05 '23 19:12 gyscos

Could SelectView::get_item_mut() solve your use-case? Or SelectView::iter_mut if you need to access several items. In particular, you should be able to swap two items efficiently this way.

I'm not sure why, but I couldn't seem to get a mutable reference properly where the compiler allowed me to mutate the references. Then I realized that I didn't care about the old values and just need an efficient way to replace an item.

Wondering if it should be set_item vs replace_item

Great idea. I'm happy to make this change.

also, we could return the old value? 🤷

I like this as it feels consistent with other set_item type APIs. I gave it a shot, but it turns out to be a bit complicated to return the values within the Rc. I've still not really used Rc much and am running into many issues with working with the type and the compiler. If you have a chance, would you mind adding a suggestion in the diff so I could see how to do this?

squaresurf avatar Dec 07 '23 00:12 squaresurf

I'm fine adding this method, if only for the S: Into<StyledString> convenience factor.

Do you mind explaining what you mean by this convenience? I'm not sure I understand this.

squaresurf avatar Dec 07 '23 00:12 squaresurf