Skip to content
Permalink
Browse files

Cleanup code, fix indentation, add "skipped expressions" dialog

  • Loading branch information
suricactus committed Mar 11, 2020
1 parent 0a68ef8 commit 368c297e583fd65442c6c4df9d66ed75e979415a
Showing with 40 additions and 24 deletions.
  1. +37 −21 src/gui/qgsexpressionbuilderwidget.cpp
  2. +3 −3 src/gui/qgsexpressionbuilderwidget.h
@@ -652,7 +652,7 @@ void QgsExpressionBuilderWidget::loadRecent( const QString &collection )
}
}

// this is potentially very slow if there are thousands of user expressions, everytime entire cleanup and load
// this is potentially very slow if there are thousands of user expressions, every time entire cleanup and load
void QgsExpressionBuilderWidget::loadUserExpressions( )
{
// Cleanup
@@ -1568,7 +1568,7 @@ void QgsExpressionBuilderWidget::loadExpressionsFromJson( const QJsonDocument &e
}

// we store the number of
QStringList skippedExpressions;
QStringList skippedExpressionLabels;
bool isApplyToAll = false;
bool isOkToOverwrite = false;

@@ -1578,12 +1578,11 @@ void QgsExpressionBuilderWidget::loadExpressionsFromJson( const QJsonDocument &e

for ( const QJsonValue &expressionValue : expressionsObject["expressions"].toArray() )
{
QgsLogger::warning( "" + QStringLiteral( __FILE__ ) + ": " + QString::number( __LINE__ ) );
// validate the type of the array element, can be anything
if ( ! expressionValue.isObject() )
{
// try to stringify and put and indicator what happened
skippedExpressions.append( expressionValue.toString() );
skippedExpressionLabels.append( expressionValue.toString() );
continue;
}

@@ -1598,49 +1597,53 @@ void QgsExpressionBuilderWidget::loadExpressionsFromJson( const QJsonDocument &e
{
// try to stringify and put an indicator what happened. Try to stringify the name, if fails, go with the expression.
if ( ! expressionObj["name"].toString().isEmpty() )
skippedExpressions.append( expressionObj["name"].toString() );
skippedExpressionLabels.append( expressionObj["name"].toString() );
else
skippedExpressions.append( expressionObj["expression"].toString() );
skippedExpressionLabels.append( expressionObj["expression"].toString() );

continue;
}

// we want to import only items of type expression for now
if ( expressionObj["type"].toString() != "expression" )
{
skippedExpressions.append( expressionObj["name"].toString() );
skippedExpressionLabels.append( expressionObj["name"].toString() );
continue;
}

// we want to import only items of type expression for now
if ( expressionObj["group"].toString() != "user" )
{
skippedExpressions.append( expressionObj["name"].toString() );
skippedExpressionLabels.append( expressionObj["name"].toString() );
continue;
}

QString label = expressionObj["name"].toString();
QString expression = expressionObj["expression"].toString();
QString helpText = expressionObj["description"].toString();
const QString label = expressionObj["name"].toString();
const QString expression = expressionObj["expression"].toString();
const QString helpText = expressionObj["description"].toString();

// make sure they have valid name
if ( label.contains( "\\" ) || label.contains( "/" ) )
{
QgsLogger::warning( "" + QStringLiteral( __FILE__ ) + ": " + QString::number( __LINE__ ) );
skippedExpressions.append( expressionObj["name"].toString() );
skippedExpressionLabels.append( expressionObj["name"].toString() );
continue;
}

if ( mUserExpressionLabels.contains( label ) )
settings.beginGroup( label );
const QString oldExpression = settings.value( QStringLiteral( "expression" ) ).toString();
settings.endGroup();

// TODO would be nice to skip the cases when labels and expressions match
if ( mUserExpressionLabels.contains( label ) && expression != oldExpression )
{
if ( ! isApplyToAll )
showMessageBoxConfirmExpressionOverwrite( isApplyToAll, isOkToOverwrite, label, expression, expression );
showMessageBoxConfirmExpressionOverwrite( isApplyToAll, isOkToOverwrite, label, oldExpression, expression );

if ( isOkToOverwrite )
saveToUserExpressions( label, expression, helpText );
else
{
skippedExpressions.append( label );
skippedExpressionLabels.append( label );
continue;
}
}
@@ -1650,22 +1653,35 @@ void QgsExpressionBuilderWidget::loadExpressionsFromJson( const QJsonDocument &e
}
}

QgsLogger::warning( "" + QStringLiteral( __FILE__ ) + ": " + QString::number( __LINE__ ) );
loadUserExpressions( );

QgsLogger::warning( QString::number( skippedExpressionLabels.count() ) + QStringLiteral( __FILE__ ) + ": " + QString::number( __LINE__ ) );

if ( ! skippedExpressionLabels.isEmpty() )
{
QStringList skippedExpressionLabelsQuoted;
for ( const QString &skippedExpressionLabel : skippedExpressionLabels )
skippedExpressionLabelsQuoted.append( QString( "'%1'" ).arg( skippedExpressionLabel ) );

QMessageBox::information( this,
tr( "Skipped expressions" ),
QString( "%1\n%2" ).arg( tr( "The following expressions have been skipped:" ),
skippedExpressionLabelsQuoted.join( ", " ) ) );
}
}

void QgsExpressionBuilderWidget::showMessageBoxConfirmExpressionOverwrite(
bool &isApplyToAll,
bool &isOkToOverwrite,
const QString &label,
QString &oldExpression,
QString &newExpression )
const QString &oldExpression,
const QString &newExpression )
{
QMessageBox::StandardButtons buttons = QMessageBox::Yes | QMessageBox::YesToAll | QMessageBox::No | QMessageBox::NoToAll;
switch ( QMessageBox::question( this,
tr( "Expression override" ),
tr( "Expression overwrite" ),
tr( "The expression with label '%1' was already defined."
"The old expression \"%2\" will be overriden by \"%3\"."
"The old expression \"%2\" will be overwritten by \"%3\"."
"Are you sure you want to overwrite the expression?" ).arg( label, oldExpression, newExpression ), buttons ) )
{
case QMessageBox::NoToAll:
@@ -396,8 +396,8 @@ class GUI_EXPORT QgsExpressionBuilderWidget : public QWidget, private Ui::QgsExp

/**
* Create the expressions JSON document storing all the user expressions to be exported.
* \since QGIS 3.14
* \returns the created expressions JSON file
* \since QGIS 3.14
*/
QJsonDocument *exportUserExpressions();

@@ -410,8 +410,8 @@ class GUI_EXPORT QgsExpressionBuilderWidget : public QWidget, private Ui::QgsExp

/**
* Load and permanently store the expressions from the expressions JSON document.
* \since QGIS 3.14
* \param expressionsDocument the parsed expressions JSON file
* \since QGIS 3.14
*/
void loadExpressionsFromJson( const QJsonDocument &expressionsDocument );

@@ -427,7 +427,7 @@ class GUI_EXPORT QgsExpressionBuilderWidget : public QWidget, private Ui::QgsExp
* \param oldExpression the old expression for a given label
* \param newExpression the new expression for a given label
*/
void showMessageBoxConfirmExpressionOverwrite( bool &isApplyToAll, bool &isOkToOverwrite, const QString &label, QString &oldExpression, QString &newExpression );
void showMessageBoxConfirmExpressionOverwrite( bool &isApplyToAll, bool &isOkToOverwrite, const QString &label, const QString &oldExpression, const QString &newExpression );

/**
* Returns the list of expression items matching a \a label.

0 comments on commit 368c297

Please sign in to comment.
You can’t perform that action at this time.