Unify druid & druid-shell selection types, rm src/text/selection.rs

[cmyr]

This removes the druid selection type, and adds any API on that type that was missing to druid-shell::Selection

• progress on New TextBox & IME integration tracking issue (report new TextBox issues here!) #1652

[lord]

A few thoughts/questions:

• When I've implemented text editing before, I've modeled selections as having two h_poss, one for the active end and one for the anchor. But I'm struggling to actually find a case where this makes a difference, so it could be that these two representations are equivalent? If they are in fact equivalent, this way with just one seems maybe better? Not sure if you've thought too much about this or not.
• How would a user create a selection with an h_pos of None?
• I think the way this interacts with InputHandler::set_selection is worth documenting? (Should the input method be able to update the h_pos directly? I feel like not, right?) Similarly, maybe worth writing down if the input method can observe the h_pos, and the h_pos changes, does that result in Event::SelectionChanged, or (my vote) Event:: LayoutChanged
• If these types are unified, you'll eventually want to add affinity as well, right? If Selection continues to grow, may want to do something to limit what the input method can access, or at least what it can implicitly set through set_selection

[cmyr]

A few thoughts/questions:

• When I've implemented text editing before, I've modeled selections as having two h_poss, one for the active end and one for the anchor. But I'm struggling to actually find a case where this makes a difference, so it could be that these two representations are equivalent? If they are in fact equivalent, this way with just one seems maybe better? Not sure if you've thought too much about this or not.

This is interesting, it would not have occurred to me to track the h-pos of the anchor edge, but now that you mention it you probably want to track the hpos of the secondary cursor, if you're doing BiDi.

• How would a user create a selection with an h_pos of None?

h_pos is none by default; it is only set when there is a vertical movement.

edit: to clarify: I have added a constructor for Selection, and I'll mark it non_exhaustive to be clear.

• I think the way this interacts with InputHandler::set_selection is worth documenting? (Should the input method be able to update the h_pos directly? I feel like not, right?) Similarly, maybe worth writing down if the input method can observe the h_pos, and the h_pos changes, does that result in Event::SelectionChanged, or (my vote) Event:: LayoutChanged

All explicit setting of the selection (that are not the product of vertical motion) clears h_pos, and h_pos is not part of visible selection state. Also it should not be possible to change h_pos without also changing some other aspect of the selection? In any case I don't think h_pos interacts with IME.

• If these types are unified, you'll eventually want to add affinity as well, right? If Selection continues to grow, may want to do something to limit what the input method can access, or at least what it can implicitly set through set_selection

Yes, I was thinking of making this be our single selection type. I can't really think of a very good way to limit what is available to IME, but I do think it would be worth documenting the expectations a bit better.

Thanks for the notes, very happy to have another pair of eyes on some of this stuff!

[cmyr]

@lord i added a bunch more docs based on your suggestions, thanks! I'm going to go ahead and merge this so I can start cleaning up some of the follow-up PRs. :)

[lord]

sounds good! :)

…

On Thu, Mar 18, 2021, at 9:29 PM, Colin Rofls wrote: @lord <https://github.com/lord> i added a bunch more docs based on your suggestions, thanks! I'm going to go ahead and merge this so I can start cleaning up some of the follow-up PRs. :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1653 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPCQCQBHF2MUY4N3UH7N2LTEKSGNANCNFSM4ZJMXIGA>.