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

Make re module return nil for unmatched captures instead of empty string #1364

Closed
wants to merge 227 commits into from
Closed

Conversation

flaviut
Copy link
Contributor

@flaviut flaviut commented Jul 15, 2014

(foo)? != ((?:foo)?)

@dom96
Copy link
Contributor

dom96 commented Jul 15, 2014

This sounds like it will break a lot of code.

@Varriount
Copy link
Contributor

@dom96 I doubt that much code uses the peg module.

What could be done is create a define() symbol for the old behavior. What is the deprecation path for these things?

@dom96
Copy link
Contributor

dom96 commented Jul 15, 2014

This doesn't edit the behaviour of the pegs module.

What is the point in creating a define()? You may as well just change your code to check for nil at that point.

@flaviut flaviut closed this Jul 21, 2014
@flaviut flaviut deleted the patch-1 branch July 21, 2014 22:53
@flaviut flaviut restored the patch-1 branch July 31, 2014 13:45
@flaviut flaviut reopened this Jul 31, 2014
@flaviut
Copy link
Contributor Author

flaviut commented Jul 31, 2014

oops, I somehow closed this accdently

@dom96
Copy link
Contributor

dom96 commented Aug 13, 2014

@Araq do we want this pulled in?

onionhammer and others added 5 commits October 10, 2014 22:49
Conflicts:
	compiler/ast.nim
	compiler/nimfix/prettybase.nim
	compiler/pragmas.nim
	compiler/sempass2.nim
	doc/manual.txt
	koch.nim
	lib/pure/concurrency/threadpool.nim
	web/news.txt
@flaviut
Copy link
Contributor Author

flaviut commented Oct 19, 2014

Yes, it looks scary, but it should merge cleanly onto big-break

@flaviut
Copy link
Contributor Author

flaviut commented Oct 29, 2014

Closing in preference for a PR against the correct branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants