Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text content by cow #1107

Merged
merged 1 commit into from
Sep 21, 2022
Merged

Text content by cow #1107

merged 1 commit into from
Sep 21, 2022

Conversation

RamType0
Copy link
Contributor

@RamType0 RamType0 commented Nov 1, 2021

#1050
Replace Text.content from String to Cow<'a,str> to reduce String allocation.

@hecrj hecrj force-pushed the master branch 2 times, most recently from e6ab610 to 8b0f2e6 Compare January 19, 2022 15:04
@tema3210
Copy link

#1050 Replace Text.content from String to Cow<'a,str> to reduce String allocation.

I wounder if it's feasible to do alter TextInput message to be (String,TInputEvent) where TInputEvent contains various things that can happen with text input, like copy-paste events, char insertion at a position, etc.

Imagined API:

enum TInputEv {
   Paste( fn(&mut String) ), //this writes paste content to the string; when it's emmited message's `String` still contains old value
   Insertion{ at: Range<usize>, content: String} // range here indicates which indices of original string are going to be changed, content is payload; old value is still unchanged
  KeyPress(char) //technically a subset of previous, but this is so common that it deserves it's own variant, I think
}

@hecrj hecrj added improvement An internal improvement text widget labels Sep 21, 2022
@hecrj hecrj added this to the 0.5.0 milestone Sep 21, 2022
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙇 I have rebased the changes on top of master.

I believe we can start by changing Text::new to take a Cow and eventually expose a new helper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement An internal improvement text widget
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants