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

Upgrade regexp_parser to 2.0 #9102

Merged
merged 1 commit into from Nov 26, 2020
Merged

Conversation

knu
Copy link
Contributor

@knu knu commented Nov 26, 2020

regexp_parser 2.0 is out, which introduced char-based indices we wanted.

ammar/regexp_parser#72
ammar/regexp_parser#73

Methods we use that used to return byte-based indices now return char-based indices, so we can revert the changes made in #8989.

https://github.com/ammar/regexp_parser/blob/master/CHANGELOG.md#200---2020-11-25---janosch-m%C3%BCller


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

Methods we use that used to return byte-based indices now return
char-based indices, so we can revert the changes made in rubocop#8989.

https://github.com/ammar/regexp_parser/blob/master/CHANGELOG.md#200---2020-11-25---janosch-m%C3%BCller
@koic
Copy link
Member

koic commented Nov 26, 2020

I was just investigating this too. Thank you for the early catching up and fixing!

@koic koic merged commit e723c9b into rubocop:master Nov 26, 2020
@knu knu deleted the upgrade_regexp_parser branch November 26, 2020 02:13
@marcandre
Copy link
Contributor

Great news, thanks!

@deepj
Copy link
Contributor

deepj commented Dec 2, 2020

This is not a good change. It breaks compatibility with other important gems such ascapybara which depends on regexp_parser ~> 1.5. So I'm stuck on rubocop 1.4.0 :(

@marcandre
Copy link
Contributor

Sorry about your troubles.
There's two things I'm looking into.

  1. relaxing our dependency to allow older regexp_parser (with potential bugs with utf-8 content)...
  2. create a PR for capybara....

@marcandre
Copy link
Contributor

Update: capybara master is already updated, just not released yet.

marcandre added a commit to marcandre/rubocop that referenced this pull request Dec 2, 2020
marcandre added a commit that referenced this pull request Dec 2, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Dec 3, 2020

Thanks for looking into this. It's nice to see Capybara is using RuboCop as well. :-)

koic added a commit to koic/rubocop that referenced this pull request Dec 3, 2020
Follow rubocop#9154,
rubocop#9155,
and rubocop#9102.

This is a step towards the widespread use of regexp_parser 2.0.

RuboCop core accepts regexp_parser 1.8, but several code is already
incompatible with regexp_parser 1.8. It can cause issue because these
combination of versions.

Therefore, this PR ports code for regexp_parser 1.8 that will never be
maintained from rubocop#9102.

Implementation of this patch, code is intentionally duplicated because
it is evaluated only when the class is defined. Also, since obsoleted
code for regexp_parser 1.8 is assumed to never be maintained, the target
to be removed is clear.

To be honest, it's ugly as implementation, but I think it has the least
impact for RuboCop 1.x series users.
bbatsov pushed a commit that referenced this pull request Dec 3, 2020
Follow #9154,
#9155,
and #9102.

This is a step towards the widespread use of regexp_parser 2.0.

RuboCop core accepts regexp_parser 1.8, but several code is already
incompatible with regexp_parser 1.8. It can cause issue because these
combination of versions.

Therefore, this PR ports code for regexp_parser 1.8 that will never be
maintained from #9102.

Implementation of this patch, code is intentionally duplicated because
it is evaluated only when the class is defined. Also, since obsoleted
code for regexp_parser 1.8 is assumed to never be maintained, the target
to be removed is clear.

To be honest, it's ugly as implementation, but I think it has the least
impact for RuboCop 1.x series users.
@deepj
Copy link
Contributor

deepj commented Dec 3, 2020

@marcandre Thanks for the changes! I missed that commit.

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

5 participants