Add address validation and matching #20
Conversation
|
I'm glad to hear that
Can you link to the specification section where method and addresses are defined so I can verify the implementation? Footnotes
|
Sure! Here what I call a method address is explained under OSC Address Spaces and OSC Addresses and a message address is right below in OSC Message Dispatching and Pattern Matching. (Sorry page doesn't have chapter anchors 😞 ) What surprised me is that method addresses can't contain any wildcars, only the address in a message can. I think I've always seen this implemented the other way 😄 |
|
Sorry, the GitHub UI confused me again. I already added the review suggestions four days ago but forgot to press the request changes button. |
No worries and thank you for the excellent review! |
|
Only a quick update, I haven't forgotten about the PR and I will hopefully come back to you somewhen this week with more details. |
|
Waits on DrLuke#1 to be merged. |
A Matcher is constructed from an OSC address pattern and will use a number of precompiled regular expression to match against a concrete OSC address. Regular expression match quite well to the pattern rules given by the OSC specification and are used by other popular OSC libraries for this purpose, e.g. [python-osc][1]. I chose `nom` as a parser combinator library to parse OSC addresses and OSC address patterns reliably and efficiently. @DrLuke please do not take this as a criticism of your implementation, I am very about happy your contribution! Actually, I would be happy if _you_ could review this change so we can merge it back into your PR and eventually release this feature. My GitHub feed showed me that @D1plo1d is working on a [`no_std` feature][2] which also adds `nom`. I would be happy to receive a review from @D1plo1d as well, hoping to reduce merge conflicts with their feature. Please be kind, I haven't programmed Rust in a long time and never used `nom` before. [1]: https://github.com/attwad/python-osc [2]: https://github.com/D1plo1d/rosc/tree/feature/no_std SQUASH: Add a couple more test cases This revealed some errors, e.g. empty pattern parts are excepted like `//foo/` which should be rejected.
s/matches/matched/ Co-authored-by: Lukas <[email protected]> Remove redundant field name Co-authored-by: Lukas <[email protected]> Remove redundant .into() Co-authored-by: Lukas <[email protected]> Use take_till1 to require a non-empty match Co-authored-by: Lukas <[email protected]> Use take_till1 to require a non-empty match Co-authored-by: Lukas <[email protected]> Use take_till1 to require a non-empty match Co-authored-by: Lukas <[email protected]> Add missing import of take_till1 Co-authored-by: Lukas <[email protected]>
Simplify address matching by providing a Matcher
|
Hey 🖖🏻 Just wanted to let you know that I'm still alive and will merge this PR soon, hopefully Sunday. Sorry for the delay. |
|
Finally, it's time for a merge. This will be included with the |
Added address module with following features:
The validation is differentiates between method and message addresses, as defined by the OSC specification. Address matching uses either direct comparison or regex for addresses with wildcards.
I've tried to cover all kinds of cases with tests but I'm sure I've missed something.
👋 This is my first time contributing to a rust project, so feedback ony my code is very welcome!