-
Notifications
You must be signed in to change notification settings - Fork 435
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
Nom nom #2480
Nom nom #2480
Conversation
(I shouldn't have committed that)
And now the benchmarks, as promised. I did them by running
There are 3 benchmarks for before (regex), and 2 benchmarks for after migrating to nom At the very least, compile times didn't regress. I think on average, they slightly improved, but I haven't done a statistically rigorous test. |
Looks good! @marc0246 do you have anything more to add? |
A couple nit picks regarding overall code structure. I'm seeing This pattern: parser(input).map(|(input, x)| (input, thing(x))) Is what parser.map(thing).parse(input) Parsers are generally not prefixed with |
Thanks to marc for suggesting this
I love it, thanks! ❤️ |
I must say I really enjoyed working on this, and then getting top tier feedback. It's been a while since I got to learn so much in such a short period of time. <3 |
Update documentation to reflect any user-facing changes - in this repository.
Make sure that the changes are covered by unit-tests.
Run
cargo clippy
on the changes.Run
cargo +nightly fmt
on the changes.Please put changelog entries in the description of this Pull Request
if knowledge of this change could be valuable to users. No need to put the
entries to the changelog directly, they will be transferred to the changelog
file by maintainers right after the Pull Request merge.
Please remove any items from the template below that are not applicable.
Describe in common words what is the purpose of this change, related
Github Issues, and highlight important implementation aspects.
The regexes for parsing the vk.xml file have been replaced with equivalent nom constructions. The nom code I wrote is probably not optimal, since I was learning the library as I went along. That's also why one will find different "styles" of nom code.
Changelog: