Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix QgsExpression::dump() for functions without params and for "NOT IN"
And add tests to check if dump() creates an expression that produces the same
result as the original expression.
  • Loading branch information
m-kuhn committed Jun 16, 2015
1 parent 5389699 commit 0bf1135
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/core/qgsexpression.cpp
Expand Up @@ -2613,7 +2613,7 @@ bool QgsExpression::NodeInOperator::prepare( QgsExpression* parent, const QgsFie

QString QgsExpression::NodeInOperator::dump() const
{
return QString( "%1 IN (%2)" ).arg( mNode->dump() ).arg( mList->dump() );
return QString( "%1 %2 IN (%3)" ).arg( mNode->dump() ).arg( mNotIn ? "NOT" : "" ).arg( mList->dump() );
}

//
Expand Down Expand Up @@ -2670,7 +2670,7 @@ QString QgsExpression::NodeFunction::dump() const
{
Function* fd = Functions()[mFnIndex];
if ( fd->params() == 0 )
return fd->name(); // special column
return QString( "%1%2" ).arg( fd->name() ).arg( fd->name().startsWith( '$' ) ? "" : "()" ); // special column

This comment has been minimized.

Copy link
@m-kuhn

m-kuhn Jun 16, 2015

Author Member

@NathanW2 , @nyalldawson can you think of a better solution than this to detect if it's a constant $pi or a function pi() and either needs the brackets () to be dumped or not?

else
return QString( "%1(%2)" ).arg( fd->name() ).arg( mArgs ? mArgs->dump() : QString() ); // function
}
Expand Down
21 changes: 15 additions & 6 deletions tests/src/core/testqgsexpression.cpp
Expand Up @@ -437,13 +437,8 @@ class TestQgsExpression: public QObject
QTest::newRow( "color cmyka" ) << "color_cmyka(50,25,90,60,200)" << false << QVariant( "51,76,10,200" );
}

void evaluation()
void run_evaluation_test( QgsExpression& exp, bool evalError, QVariant& result )
{
QFETCH( QString, string );
QFETCH( bool, evalError );
QFETCH( QVariant, result );

QgsExpression exp( string );
QCOMPARE( exp.hasParserError(), false );
if ( exp.hasParserError() )
qDebug() << exp.parserErrorString();
Expand Down Expand Up @@ -501,6 +496,20 @@ class TestQgsExpression: public QObject
}
}

void evaluation()
{
QFETCH( QString, string );
QFETCH( bool, evalError );
QFETCH( QVariant, result );

QgsExpression exp( string );
run_evaluation_test( exp, evalError, result );
QgsExpression exp2( exp.dump() );
run_evaluation_test( exp2, evalError, result );
QgsExpression exp3( exp.expression() );
run_evaluation_test( exp3, evalError, result );
}

void eval_precedence()
{
QCOMPARE( QgsExpression::BinaryOperatorText[QgsExpression::boDiv], "/" );
Expand Down

0 comments on commit 0bf1135

Please sign in to comment.