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

Enable extended patterns in fnmatch #659

Closed
wants to merge 1 commit into from

Conversation

tsipinakis
Copy link
Member

@tsipinakis tsipinakis commented Aug 31, 2019

I wanted to add a compilation warning but apparently #warning is a gcc specific extension. Not sure how else we could indicate if this is enabled or not.

Fixes #645
Fixes #658

@tsipinakis tsipinakis requested a review from bebehei August 31, 2019 10:32
Copy link
Member

@bebehei bebehei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice and clean solution. But I've been getting the suspicious if this is a good solution to solve our problem. I've got absolutely no knowledge, how GLib and other libs are working when compiling multiple units with different sources and linking them.

@tsipinakis
Copy link
Member Author

I've got absolutely no knowledge, how GLib and other libs are working when compiling multiple units with different sources and linking them.

Theoretically this should not play a role at all. Either the GNU extensions exist in libc or they don't our definitions do not affect that. All it does is control if they are exported in the header to allow us to use them.

See fnmatch.h, fnmatch.c.

@tsipinakis
Copy link
Member Author

But I've been getting the suspicious if this is a good solution to solve our problem

I've been getting suspicious too but for an entirely different reason - I'm unsure if fnmatch is the best fit here. As the manpage mentions it's intended for paths and while it does what we need I feel like regex would be a better fit.

@bebehei
Copy link
Member

bebehei commented Sep 5, 2019

To achieve negative matching, I guess we either have to implement such "quirk" and document it right or allow regexes.

TBH, I'd prefer the fnmatch thing ... if it wouldn't be such a weird mess.


Just to evaluate a possible regex path: How would we implement it? While thinking about it, I guess,. you had some ideas. We can't have it as a drop-in replacement. Chars like .?\()[] would have a complete different meaning now.

@tsipinakis
Copy link
Member Author

We can't have it as a drop-in replacement. Chars like .?()[] would have a complete different meaning now.

I was thinking using something like a tilde as a prefix for it, so appname = "firefox" uses fnmatch and `appname = "~firefox" uses regex.

@bebehei
Copy link
Member

bebehei commented Sep 7, 2019

I was thinking using something like a tilde as a prefix for it, so appname = "firefox" uses fnmatch and `appname = "~firefox" uses regex.

What about a slash? I guess it's more intuitive.

@tsipinakis
Copy link
Member Author

I'll close this as, as said above it's better to add an option to use regex rather than enable extended patterns.

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.

applying new_icon to one rules changes for all There is a way to enable extglob in dunstrc file
2 participants