-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
Unify and Improve error handling of restricted strings #269
Conversation
Actually error reporting isn't that good. Might be better to give the failed value also in the error message like in other codecs |
Yes, hough I think it might be good to have a unified error across all codecs for strings, so this logic can be better shared, and if codecs can provide more context they can wrap the error type. |
src/types/strings/constrained.rs
Outdated
@@ -10,6 +10,7 @@ pub(crate) trait StaticPermittedAlphabet: Sized + Default { | |||
const CHARACTER_SET: &'static [u32]; | |||
const CHARACTER_WIDTH: u32 = crate::num::log2(Self::CHARACTER_SET.len() as i128); | |||
|
|||
fn alphabet_name() -> &'static str; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be better as a enum, and have it as a associated const instead of a fn.
Related to this #185, maybe it can be handled in the same time. |
Thank you for your PR! Looks good to me, I notice a lot of commented out code, can that code be removed or is there a reason it's commented out? |
I was just finishing it, some small cleaning still to do. I unified all the string types, so that they implemented I had to wrap all the errors behind Also all |
Invalid restricted string error, for example, should look something like this now:
|
I think it is ready now. Not sure what is the performance since all characters on restricted strings are verified now in every codec. |
Thank you for your PR! |
Fixes #260
Adds
alphabet_name
function forStaticPermittedAlphabet
trait for better error messages.