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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[expression] strpos() and regexp_match() improvements #3732

Merged
merged 1 commit into from
Nov 10, 2016

Conversation

nirvn
Copy link
Contributor

@nirvn nirvn commented Nov 8, 2016

This PR implements what was discussed in the dev mailing list yesterday (see https://lists.osgeo.org/pipermail/qgis-developer/2016-November/045703.html), namely:

  • strpos() now relies on a simple string within a string search
  • regexp_match() now returns pos of a matching regular expression

This change means that users using expressions such as if(regexp_match('blah','blah') = true, ..., ...) won't work as expected anymore, and would have to be re-written to if(regexp_match('blah','blah'), ..., ...).

IMHO, since we're allowing ourselves to break stuff prior to release of version 3.0, this expression-breaking improvement is warranted 馃槃

@nirvn nirvn force-pushed the strpos_regexp_match_upgrade branch from 7e2106c to 6627183 Compare November 8, 2016 05:29
@nyalldawson
Copy link
Collaborator

Logic sounds good to me... could do with a unit test for the new strpos behaviour, ie searching for '.'

@nirvn nirvn force-pushed the strpos_regexp_match_upgrade branch from 6627183 to ff777b2 Compare November 8, 2016 05:38
@nirvn
Copy link
Contributor Author

nirvn commented Nov 8, 2016

@nyalldawson , test added; feel free to cancel the previously pending travis build check.

@nyalldawson
Copy link
Collaborator

Sorry - one last thing. Can you change the commit message to a [FEATURE] so that the documentation team gets pinged and also so that we pick this change up and include it in the release notes.

- strpos() now relies on a simple string within a string search
- regexp_match() now returns pos of a matching regular expression
@nirvn nirvn force-pushed the strpos_regexp_match_upgrade branch from ff777b2 to 6b2b4c5 Compare November 10, 2016 04:55
@nirvn
Copy link
Contributor Author

nirvn commented Nov 10, 2016

@nyalldawson done.

@nyalldawson nyalldawson merged commit 6ea0049 into qgis:master Nov 10, 2016
@nirvn nirvn deleted the strpos_regexp_match_upgrade branch February 26, 2018 12:40
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.

None yet

2 participants