-
Notifications
You must be signed in to change notification settings - Fork 373
Add support for PCRE #1536
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
base: master
Are you sure you want to change the base?
Add support for PCRE #1536
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1536 +/- ##
==========================================
+ Coverage 65.37% 65.42% +0.05%
==========================================
Files 51 51
Lines 8944 8957 +13
Branches 1069 1071 +2
==========================================
+ Hits 5847 5860 +13
Misses 3097 3097
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/extra-ci |
|
@fwsmit let me know what do you think of this |
|
I think it would be better to have a regex type config variable instead if multiple booleans that have an arbitrary priority. |
The best is undoubtedly PCRE as it allows many things like negative matches, lookahead ecc. In an ideal world I would remove the posix regex boolean and just use this. However the config is literally called So mainly it's backwards compatibility. Another option is to add an option like Let me know what do you think it's best moving forward |
|
@dunst-project/developers thoughts? |
|
Is pcre regex backward compatible with the current posix regex? |
|
as far as I know it's not 100% backwards compatible. that's why I didn't silently upgrade enable_posix_regex to use GRegex implementation. |
|
Main difference between PCRE and non-PCRE is the escaping of braces. One needs to escape them to have special meaning, the other you have to escape to match a brace literal. This sucks. It's like a USB-A connector. Correct at the For the rest, PCRE is mostly an upgrade. BTW: you're allowed to release dunst 2.0 and drop the unwanted code. So you could drop ERE support here and switch to PCRE. |
Yes the idea was to provide "experimental" PCRE support and in v2 just phase out ERE. |
|
I thought Regex is for devs definitely more flexible, but more complex for entry level linux users. You do the decision. I can go with both. |
Glib exposes PCRE with GRegex. Since we already have the library lying around we might as well use it