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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow options to be passed when processing a String pattern #68

Merged
merged 2 commits into from Sep 20, 2020

Conversation

owst
Copy link
Contributor

@owst owst commented Sep 12, 2020

@jaynetics as mentioned in rubocop/rubocop#8625 (comment) - please let me know what you think 馃憤

@jaynetics
Copy link
Collaborator

@owst thank you, this looks nice!

the only thing we might want to change is how the passed options are silently ignored when used alongside a Regexp. this seems a little counterintuitive. i'd either let it raise an error, or we can simply override the options from the Regexp if people do try to pass their own options.

@ammar this code uses kwargs, which would mean dropping support for running on 1.9. are you fine with that?

@owst
Copy link
Contributor Author

owst commented Sep 13, 2020

@jaynetics yeah, good point; I think raising is best (for now)

@owst
Copy link
Contributor Author

owst commented Sep 16, 2020

Hi @jaynetics just checking-in on this, do you have any further thoughts? I'd love to get this merged and for a new version of regexp_parser to be released so I can progress rubocop/rubocop#8625

Thanks!

@jaynetics jaynetics merged commit d109b56 into ammar:master Sep 20, 2020
@jaynetics
Copy link
Collaborator

@ammar FYI

i have merged this PR which ends support for Ruby 1.9.x and will update the README accordingly.

i could not find any dependents which seem to require it.

if it turns out you or anyone else really still need such support i suggest we release another version to restore it.

@jaynetics
Copy link
Collaborator

@owst released with v1.8.0, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants