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

Use generics and macros #274

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Use generics and macros #274

merged 2 commits into from
Oct 10, 2023

Conversation

future-highway
Copy link
Contributor

This also implements #271 on next which is included in the code changes related to HasLen, though I don't see how HasLen is used in this branch.

Most of the code was eliminated by using generic implementations for Option<T> and &T.

@pintariching
Copy link
Contributor

I don't think the trait HasLen is used anywhere. I think I forgot to remove it in the rewrite.

@Keats
Copy link
Owner

Keats commented Oct 7, 2023

So for the length validator, I would like to add an option to the validator for the type of length:

  • chars (default)
  • utf8 bytes
  • utf16 bytes (used in HTML)

So you would call it like:

#[validate(length(min = 1)]. 
#[validate(length(min = 1, kind="chars")]   // equal to previous line
#[validate(length(min = 1, kind="utf8_bytes")]
#[validate(length(min = 1, kind="utf16_bytes")]

What do people think? Not on this PR though, just to get some feedback while talking about it.

@Keats
Copy link
Owner

Keats commented Oct 7, 2023

And I guess we can remove all the HasLen handling if it's unused

@future-highway
Copy link
Contributor Author

I don't have a strong opinion on the type of length since I'm not actually using this library anymore (I was wrapping the lib in my own validation library and HasLen was my main use, so I never took full advantage of the features).

My only thought is that it may be an easy way to make a mistake by forgetting to specify. Maybe having distinct choices (e.g., length_in_u8, length_in_chars, etc.) would prevent such a mistake. count could be used for Vec and [T] to distinguish them.

As I said though, I'm not using this library currently and it's probably a better idea to take the input of others.

I'll update the PR to delete HasLen

Plus a couple of clippy suggestions
@future-highway
Copy link
Contributor Author

PR updated.

May also want to consider replacing the use of lazy_static (see: rust-lang-nursery/lazy-static.rs#214) with once_cell which also has a lazy interface. I didn't think I should mix it in this PR.

@Keats
Copy link
Owner

Keats commented Oct 10, 2023

Yes, lazy static will be replaced later, thanks!

@Keats Keats merged commit d3455b8 into Keats:next Oct 10, 2023
4 checks passed
Keats pushed a commit that referenced this pull request Mar 4, 2024
* Reduce boilerplate code using generics and macros

* Remove `HasLen`

Plus a couple of clippy suggestions
Keats pushed a commit that referenced this pull request Mar 4, 2024
* Reduce boilerplate code using generics and macros

* Remove `HasLen`

Plus a couple of clippy suggestions
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.

3 participants