-
Notifications
You must be signed in to change notification settings - Fork 2
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
I went a little bit ahead of myself... #22
base: master
Are you sure you want to change the base?
Conversation
That way, we have nice error messages instead of panics if they get the patterns wrong.
Not that it would matter, but IO is really costly if not done correctly. Ideally, one buffers separately, if it ever becomes an issue.
There error handling could easily be added.
pattern, | ||
}) | ||
}) | ||
.collect::<Result<_, _>>()?, |
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.
This is one of the best tricks in the book. Collect into a Result<_, _>
, which is useful if your iterators produces Result<T, _>
. That way, if there is an error, you will get it into the top level. You get a pristine Vec<T>
.
It shows that `.as_ref()` is something the checker doesn't really see through, leading to issues which should be none. Reminder to myself: use 'ref' based destructuring preferably over things like 'Option::as_ref()`.
human-panic = "1.0.1" | ||
lazy_static = "1.3.0" |
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.
maybe you can use OnceCell instead of lazy_static I guess
... and thought I could make it a nice practice to write it as idiomatically as possible.
In the process, I changed a lot, but I believe to the better.
I hope you find these changes useful, in any case it was fun writing some Rust again :).