egui icon indicating copy to clipboard operation
egui copied to clipboard

Allow TextEdit to represent and validate generic types

Open tye-exe opened this issue 2 months ago • 9 comments

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:

  • [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 based TextEdits not execute any display string code.
  • 2 Create a method for TextType that specifies whether a given datatype should use a display string .

TODO

  • [x] Enable temporarily allowing invalid states (See Allowing Invalid States).
  • [x] Implement TextType as 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 TextType attribute in TextEdit should use generics or dynamic dispatch.
  • [ ] Discuss TextBuffer status 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.

tye-exe avatar Oct 09 '25 13:10 tye-exe

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

github-actions[bot] avatar Oct 09 '25 13:10 github-actions[bot]

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.

Mingun avatar Oct 09 '25 13:10 Mingun

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>>

tye-exe avatar Oct 09 '25 14:10 tye-exe

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).

Mingun avatar Oct 09 '25 15:10 Mingun

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.

tye-exe avatar Oct 09 '25 20:10 tye-exe

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

tye-exe avatar Oct 10 '25 10:10 tye-exe

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.

tye-exe avatar Oct 10 '25 11:10 tye-exe

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.

tye-exe avatar Oct 20 '25 12:10 tye-exe

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.

tye-exe avatar Oct 27 '25 13:10 tye-exe