Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FEATURE] expression: align strpos behaviour with postgres
  • Loading branch information
jef-n committed Nov 19, 2015
1 parent f3a5dcc commit 2a557db
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/core/qgsexpression.cpp
Expand Up @@ -1012,7 +1012,7 @@ static QVariant fcnConcat( const QVariantList& values, const QgsExpressionContex
static QVariant fcnStrpos( const QVariantList& values, const QgsExpressionContext*, QgsExpression *parent )
{
QString string = getStringValue( values.at( 0 ), parent );
return string.indexOf( QRegExp( getStringValue( values.at( 1 ), parent ) ) );
return string.indexOf( QRegExp( getStringValue( values.at( 1 ), parent ) ) ) + 1;
}

static QVariant fcnRight( const QVariantList& values, const QgsExpressionContext*, QgsExpression *parent )
Expand Down
4 changes: 2 additions & 2 deletions tests/src/core/testqgsexpression.cpp
Expand Up @@ -484,8 +484,8 @@ class TestQgsExpression: public QObject
QTest::newRow( "regexp_substr" ) << "regexp_substr('abc123','(\\\\d+)')" << false << QVariant( "123" );
QTest::newRow( "regexp_substr no hit" ) << "regexp_substr('abcdef','(\\\\d+)')" << false << QVariant( "" );
QTest::newRow( "regexp_substr invalid" ) << "regexp_substr('abc123','([[[')" << true << QVariant();
QTest::newRow( "strpos" ) << "strpos('Hello World','World')" << false << QVariant( 6 );
QTest::newRow( "strpos outside" ) << "strpos('Hello World','blah')" << false << QVariant( -1 );
QTest::newRow( "strpos" ) << "strpos('Hello World','World')" << false << QVariant( 7 );
QTest::newRow( "strpos outside" ) << "strpos('Hello World','blah')" << false << QVariant( 0 );
QTest::newRow( "left" ) << "left('Hello World',5)" << false << QVariant( "Hello" );
QTest::newRow( "right" ) << "right('Hello World', 5)" << false << QVariant( "World" );
QTest::newRow( "rpad" ) << "rpad('Hello', 10, 'x')" << false << QVariant( "Helloxxxxx" );
Expand Down

4 comments on commit 2a557db

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with the new behaviour, but how will we handle older projects? A warning in the release notes?

@jef-n
Copy link
Member Author

@jef-n jef-n commented on 2a557db Nov 19, 2015

Choose a reason for hiding this comment

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

If we had release notes - I'd say we should add it to the visual changelog (and I added a [FEATURE] to make the documentation team aware - and the lazy coder added a comment to the new issue qgis/QGIS-Documentation#679)

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 2a557db Nov 19, 2015

Choose a reason for hiding this comment

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

Who did the lazy coder script? Victim identified and contacted.
It would be nice to enhance it so it catches deprecated annotations and puts it into the "release notes".

@nirvn
Copy link
Contributor

@nirvn nirvn commented on 2a557db Nov 20, 2015

Choose a reason for hiding this comment

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

@jef-n , that was quick, thanks. BTW, the strpos function help text needs updating too.

Please sign in to comment.