Skip to content

Changed TextInputFilter from enum to closure - #25

Closed
eclipse-909 wants to merge 3 commits into
ickshonpe:masterfrom
eclipse-909:master
Closed

Changed TextInputFilter from enum to closure#25
eclipse-909 wants to merge 3 commits into
ickshonpe:masterfrom
eclipse-909:master

Conversation

@eclipse-909

@eclipse-909 eclipse-909 commented Jul 28, 2025

Copy link
Copy Markdown

This PR introduces a breaking change to the library.

TextInputFilter is limited to 3 pre-defined filters. The switch to directly using regex allows implementer to specify a custom filter.

You could add a TextInputFilter::Custom(Regex) variant in the TextInputFilter enum as a non-breaking change, but then it just becomes a regex wrapper with 3 pre-defined regexes, and it has more overhead due to pattern matching at runtime. It would be better to have const or static variables for pre-defined filters.

However, I removed the lazy-static regexes for integer, decimal, and hex because I think it's better to let the implementer decide which regexes they want to use. Getting rid of them also removes the once-cell dependency and the marginal cost of having static variables that you might not use.

The library and all modified examples were built locally with no errors.

Edit: Instead of forcing a regex, I've changed it to a closure for better generalization.

@eclipse-909

Copy link
Copy Markdown
Author

This PR conflicts with PR #24. It's your library and your repository, but I would not merge that PR because it doesn't give you the option to make a custom filter.

@elijahimmer

Copy link
Copy Markdown

I think it would be far better to just let the user provide a validator function, which they may optionally use regex instead of forcing regex and the performance penalties and complexities of it.

So the filter would just take a predicate function fn (&str) -> bool and which they may do whatever inside.
We can they have predefined predicates (like the current ones filters) and not force regex.

@elijahimmer

Copy link
Copy Markdown

That also means that the filters won't have to be static, which may be a problem in some cases that could just be avoided.

@eclipse-909

Copy link
Copy Markdown
Author

I like that idea better. I'll change that in a bit. Thanks!

@elijahimmer

Copy link
Copy Markdown

I'm writing the PR now

@elijahimmer

Copy link
Copy Markdown

#26 is the PR where I added it based on my prior work.

@eclipse-909 eclipse-909 changed the title changed filter from TextInputFilter to Regex Changed TextInputFilter from enum to closure Jul 28, 2025
@eclipse-909

eclipse-909 commented Jul 28, 2025

Copy link
Copy Markdown
Author

I have changed it to using a closure instead of a regex, removing the regex dependency as well. This PR also conflicts with PR #26. I still think this approach is better because it eliminates and unnecessary wrapper.

@eclipse-909 eclipse-909 closed this by deleting the head repository Feb 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants