Skip to content
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

Add block support to Lexer.find_fancy #1195

Closed
wants to merge 3 commits into from

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Jun 18, 2019

Discussion is intended to continue in #1188.

Currently, revisions to Lexer are being discussed in #1188. This PR is intended to improve the solution provided in that branch.

The code proposed in #1188 includes a method Lexer.lookup_fancy that returns a Lexer subclass. This is in addition to the existing method Lexer.find_fancy that returns an instance of a Lexer subclass. This difference is not evident from the names of the methods and so this commit renames lookup_fancy to find_fancy_class.

Separately, the commit also adds support to consumers passing a block to find_fancy. It is not envisaged that block passing will be common but supporting it allows for consumers to use idiomatic Ruby to customise the instantiation of a lexer.

Finally, tests are added.

http://jneen.net/ and others added 3 commits June 14, 2019 06:28
The method `Lexer.lookup_fancy` returns a Lexer subclass while the
method `Lexer.find_fancy` returns an instance of the Lexer subclass.
This is not evident from the name and so this commit renames
`lookup_fancy` to `find_fancy_class`.

Separately, the commit also adds support to consumers passing a block to
`find_fancy`. It is not envisaged that block passing will be common.

Finally, test are added.
@pyrmont pyrmont added the dependent-pr The PR is dependent on other changes being merged and will be reviewed later. label Jun 18, 2019
@pyrmont pyrmont force-pushed the feature.lexer-with branch from 3c54f92 to 451b978 Compare July 12, 2019 18:25
@pyrmont pyrmont closed this Jan 8, 2020
@pyrmont pyrmont deleted the prfix.pr-1188 branch January 8, 2020 20:06
@pyrmont pyrmont restored the prfix.pr-1188 branch January 8, 2020 20:15
@pyrmont pyrmont reopened this Jan 8, 2020
@tancnle
Copy link
Collaborator

tancnle commented Dec 4, 2022

👋🏼 @pyrmont There are some overlaps between this PR and #1565 which is already merged. Do you know if this is still relevant? 🙏🏼

@pyrmont
Copy link
Contributor Author

pyrmont commented Dec 4, 2022

@tancnle I'm afraid I don't remember any more :( Without having looked at the PRs in detail, I could imagine that there are still some ideas in this code that would make the API slightly more consistent but if this has been outstanding for the last three years, it's probably a safe bet that the changes weren't that important :D

@tancnle
Copy link
Collaborator

tancnle commented Dec 4, 2022

Thanks for the prompt respond, @pyrmont. I will close this PR then. Thank you for your work on this 🙇🏼

@tancnle tancnle closed this Dec 4, 2022
@pyrmont pyrmont deleted the prfix.pr-1188 branch December 4, 2022 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependent-pr The PR is dependent on other changes being merged and will be reviewed later.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants