Skip to content

Commit

Permalink
Fix QgsExpression::dump() for functions without params and for "NOT IN"
Browse files Browse the repository at this point in the history
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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.