Allow TextEdit to represent and validate generic types
Allow any type implementing the required trait (TextType) to be displayed and validated by TextEdit.
I am not sure whether the example added for this PR should remain, but other than that, this PR is ready for merging.
Closes:
- https://github.com/emilk/egui/issues/7564
- https://github.com/emilk/egui/issues/1348
- https://github.com/emilk/egui/pull/1349
- [x] I have followed the instructions in the PR template
Previously
This is still a work in progress. I am opening this as a draft to get feedback on:
- 1 Is this a change that is still desired
- 2 Implementation details
Example
I have added an example at ./examples/text_validation/. This gives a brief showcase towards what the functionality and API of this more generic TextEdit might look like.
Implementation Details
TextEdit has been changed to take a generic parameter that is bounded by TextType.
TextType requires two functions, one to convert the value into a string, the other to convert the value from a string. I could not use the existing ToString and FromString because FromString is not required to parse the output from ToString.
Users can define custom validation rules by implementing TextType on new types.
Allowing Invalid States (Implemented)
Currently the TextEdit can only exist in valid states which is not desirable. A simple example of this is the char data type. The only way for a user to edit it currently is for the user to write the new char elsewhere, and then paste it over the existing character. This occurs because a char can only contain one character, so if the user attempts to add their new desired character the TextEdit becomes invalid and the new character is removed, since there was more than one character. The same sequence occurs if the user attempts to delete the character to add a new one.
My idea to resolve this problem is to allow invalid states while a TextEdit is focused, which will be implemented by storing the string representation of the TextEdit value using TextEditState (this will require adding a new attribute to TextEditState).
While Focused: any valid changes will be written to both the string and the represented value.
While Focused: any invalid changes will be written only to the string.
If the TextEdit looses focus while in an invalid state, a valid string representation will be set from the represented value.
This implementation will require all TextEdits that are mutable to use a persistent Id to store the string representation between frames (certain datatypes could be made exempt, See String Optimisations).
String Optimisations
I have not conducted research into which types will be used most often, but I am going to guess that the most common datatype will be &mut String (and other equivalents). Without any optimisation for the raw string datatypes, the TextEdit will be storing two copies, one as the display string and one as the represented value. This is wasteful.
The idea that I have to combat this issue (if it is needed) is to skip using the display string for unvalidated string data types. This can be accomplished in multiple ways. The two that come to mind are:
- 1 Using generics (
impl TextEdit<String>) to have string basedTextEdits not execute any display string code. - 2 Create a method for
TextTypethat specifies whether a given datatype should use a display string .
TODO
- [x] Enable temporarily allowing invalid states (See Allowing Invalid States).
- [x] Implement
TextTypeas both mutable and immutable for core datatypes, such as strings and integers. - [x] Add comments.
- [x] Add tests(?).
- [x] Publicly export
TextType. - [ ] Discuss whether the
TextTypeattribute inTextEditshould use generics or dynamic dispatch. - [ ] Discuss
TextBufferstatus in regards to backwards compatibility. - [ ] Discuss temporarily allowing invalid states (See Allowing Invalid States).
- [ ] Discuss optimisations for string representation (See String Optimisations).
- [ ] Discuss variable, type, method, and file names.
All todo goals are ones that I can achieve without external help, I just want to know whether any of this is desired before I dedicate more time.
Preview available at https://egui-pr-preview.github.io/pr/7621-textedit Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.
View snapshot changes at kitdiff
Without criticizing the approach itself, I would like to note that in order to solve the original problem with char, the most logical thing would be to change the editing principle for this type: automatically replace the value with the last character typed / pasted. Thus, there will never be a situation where there will be 2 characters or something needs to be erased.
automatically replace the value with the last character typed / pasted.
If you mean in regards to current system, to implement that I could change the read_from_string function to take the current value and the display string. This would be useful for situations like the char implementation, so the implementation can deduce the character was newly added, though I cannot think of any other situations that it may be useful off the top of my head.
E.G. fn read_from_string(&self, displayed: &str) -> Option<Result<Self, Self::Err>>
I mean, the typed characters usually appended to the text input, which creates difficulties for the char (because it expects exactly one character). For it the character in text box should be replaced without requiring to manually select it (as you do when you want to replace part of string).
I've updated the read_from_string(s: &str) function to read_from_strings(previous: &str, modified: &str). This allows the char parsing to detect which characters have been modified and behave accordingly.
Regarding the todo of Clean up TextBuffer implementations to apply directly to String type.
I am wondering whether it will be better to switch TextBuffer to a newtype around &mut str or to leave it as an interface applying to the String datatype.
Changing it to a newtype will prevent the current implementations from being applied to all instances of String, which could more properly scope the functions. However, this would be a breaking change.
Leaving it as a trait that is implemented by String will reduce/eliminate breaking changes depending on the method taken. Breaking changes depend on if &dyn TextBuffer is replaced with &str in public interfaces, such as layouter in TextEdit
Whilst working on "Allowing Invalid States", I came encountered the following issue and its corresponding PR:
As suggested by willbicks in his response to the issue, combining lost_focus() with clicked_elsewhere() allowed for proper detection of focus loss.
I would like feedback on whether TextType::is_mutable should be renamed to TextType::is_parseable.
This would be to better represent the true purpose of the function, which is a performance optimisation of TextType::read_from_string().is_none(). Furthermore, there may be multiple types that cannot be parsed from a string (I think that these exist, but I cannot think of any examples).
React with :+1: or :-1:. :+1: - supporting this change. :-1: - opposing this change. Or comment with a more nuanced take.
After looking through more of the TextEdit code, the TextEditUndoer in the TextEditState creates a clone of the TextEdit string for each change that is made. Therefore, attempting to implement optimisations specifically for strings is likely negligible.