Conversation
…) == find_all()[0]
…dling of normalized_method_name()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, widlparser has
Parser.normalized_method_name(), which will search for a method based on the args (but doesn't understand theFoo/barpath syntax), andParser.find(), which understands the path syntax but ignores arguments.I was trying to fix speced/bikeshed#1192, and realized the issue was that I was relying on both of these methods having both of the abilities, meaning I was hitting error cases both times!
This PR fixes three things I ran into while debugging this:
normalized_method_name, if you don't manually pass the interface name as a separate argument. That is,Foo/bar(arg)will now work, rather than falling down to the final error case because there's no method namedFoo/bar.find()andfind_all()use a consistent ordering. Not strictly required for this, but it bugged me thatfind()gave the last result in the parser whilefind_all()gave them in order. Generally,foo()should be equivalent tofoo_all()[0]. I can snip this commit out if you really want.find_method()(andfind_methods()) method toParser, which uses the full parser-normalization and argument usage ofnormalized_method_name, but returns the actual matching construct.I'm considering making
find()andfind_all()dispatch tofind_method()if it finds any parentheses, and only doing its current "remove/ignore parens and just look for a name" if that fails to find anything. I haven't added that in yet, tho.I also haven't run the linter yet, apologies. Just wanted to get the patch up for an eye-over before I left for the day.