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

Regex dependency #66

Merged
merged 5 commits into from
Sep 30, 2018
Merged

Regex dependency #66

merged 5 commits into from
Sep 30, 2018

Conversation

ihh
Copy link
Member

@ihh ihh commented Sep 30, 2018

Allows matching of Perl-style regexes in Prolog blocks, via https://github.com/mndrix/regex

test_$(ABC): { =~(ABC,'^a') }
	echo Fired rule for $@ >$@

This will fire for test_apple but not test_cat or test_APPLE.

Introduces external dependency on https://github.com/mndrix/regex

@cmungall
Copy link
Member

This is a showstopper: mndrix/regex#7

We can use http://www.swi-prolog.org/pldoc/doc_for?object=section(%27packages/pcre.html%27) as suggested in the ticket. This is now bundled by default in the stable 7.6.x series. Some users may need to upgrade their swi-prolog installation.

@cmungall
Copy link
Member

I'm being dense, I can't figure out how the regex test passes, if ^ doesn't work.. it looks like your PR introduces the dependency but doesn't actually extend the grammar, so the test passes by an accident of mis-parsing...?

@ihh
Copy link
Member Author

ihh commented Sep 30, 2018

The test passes because, I think, the prefix notation =~(TEXT,REGEX) works but the infix notation TEXT =~ REGEX doesn't, or maybe it's something about quoting the REGEX when it begins with a caret... not sure... anyway it's moot because I actually wanted to go with pcre (I tried it first and failed), I should have checked with a more recent build. I've now amended the PR to use pcre, see what you think.

@cmungall
Copy link
Member

Looks great

And doh, I was scanning the gnumake_parser code for =~, but I forgot that the mndrix lib used infix and that {...} executed prolog, all makes sense now

@ihh
Copy link
Member Author

ihh commented Sep 30, 2018

btw the modifications to queue.pl have nothing to do with the regex extension, they're about implementing the $(SHELL) variable (see #54), I rolled them into this branch by accident...

@ihh ihh merged commit 19bf182 into master Sep 30, 2018
@ihh ihh deleted the regex branch September 30, 2018 23:46
@ihh
Copy link
Member Author

ihh commented Sep 30, 2018

for the record, the syntax is now (in line with PCRE)

test_$(ABC): { re_match(ABC,'^a') }
	echo Fired rule for $@ >$@

Goodbye to the =~ operator...

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.

2 participants