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 pcre2 ripgrep option #1095

Merged
merged 1 commit into from Jul 3, 2019

Conversation

@dwelle
Copy link
Contributor

commented Jun 30, 2019

Description of the Change

Added option to enable pcre2 ripgrep regex engine, as discussed in here.

NOTE: I haven't written any specs because I'm not sure what steps to take. Should I wrap all the specs in additional loop, same as prev PR has done with ripgrep support?

NOTE: cannot build Atom ATM so haven't tested the changes locally.

Benefits

Allows additional regex features such as lookbehind.

Possible Drawbacks

Enabling PCRE2 potentially incurs performance penalty, as explained on ripgrep's wiki, thus this PR does not enable it by default, and instead adds an option.

Applicable Issues

Addresses #571

Accompanying PR in atom core

@rafeca

rafeca approved these changes Jul 1, 2019

Copy link
Contributor

left a comment

Thanks! I've done some basic testing and this PR seems to be working fine. Once the PR on atom/atom gets merged we can ship this one.

@dwelle

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

Just reminding that this can now be merged, too.

@rafeca

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

Thanks for the reminder 😃

@rafeca rafeca merged commit 9757429 into atom:master Jul 3, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dwelle

This comment has been minimized.

Copy link
Contributor Author

commented Jul 13, 2019

Mhm, seems it didn't work. I have both ripgrep and PCRE2 option enabled, but the parser complains the regex is invalid. Is that coming from ripgrep or atom? How come specs passed?

image

EDIT: hold on a minute. I've updated find-and-replace, but haven't updated core (still on stable). That must be it. So actually we should have waited with merging this PR, after all :)

@Arcanemagus

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

The update of find-and-replace hasn't hit the beta channel yet, you can test it out on the nightly releases now, or wait for it to hit beta/stable during the normal release cycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.