Skip to content

Commit

Permalink
[dxf] Address review: prefer overridden layer name over layer title (…
Browse files Browse the repository at this point in the history
…the former can be changed in the export dialog, whereas the latter must be done in the layer properties, therefore, overridden layer name is closer to the export intention and should take the precedence)
  • Loading branch information
gacarrillor committed Apr 16, 2024
1 parent c2e248b commit 82fb6cc
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 17 deletions.
4 changes: 2 additions & 2 deletions src/analysis/processing/qgsalgorithmdxfexport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ QString QgsDxfExportAlgorithm::groupId() const
QString QgsDxfExportAlgorithm::shortHelpString() const
{
return QObject::tr( "Exports layers to DXF file. For each layer, you can choose a field whose values are used to split features in generated destination layers in the DXF output.\n\n"
"If no field is chosen, you can still override the output layer name by preferring layer title (set in layer properties) or by directly entering a new output layer name in the Configure Layer panel." );
"If no field is chosen, you can still override the output layer name by directly entering a new output layer name in the Configure Layer panel or by preferring layer title (set in layer properties) to layer name." );
}

QgsDxfExportAlgorithm *QgsDxfExportAlgorithm::createInstance() const
Expand All @@ -72,7 +72,7 @@ void QgsDxfExportAlgorithm::initAlgorithm( const QVariantMap & )
addParameter( extentParam.release() );
addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "SELECTED_FEATURES_ONLY" ), QObject::tr( "Use only selected features" ), false ) );
std::unique_ptr<QgsProcessingParameterBoolean> useTitleParam = std::make_unique<QgsProcessingParameterBoolean>( QStringLiteral( "USE_LAYER_TITLE" ), QObject::tr( "Use layer title as name" ), false );
useTitleParam->setHelp( QObject::tr( "If no attribute is chosen, prefer layer title (set in layer properties) to layer name" ) );
useTitleParam->setHelp( QObject::tr( "If no attribute is chosen and layer name is not being overridden, prefer layer title (set in layer properties) to layer name" ) );
addParameter( useTitleParam.release() );
addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "FORCE_2D" ), QObject::tr( "Force 2D output" ), false ) );
addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "MTEXT" ), QObject::tr( "Export labels as MTEXT elements" ), true ) );
Expand Down
12 changes: 10 additions & 2 deletions src/core/dxf/qgsdxfexport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2391,10 +2391,18 @@ QStringList QgsDxfExport::encodings()
QString QgsDxfExport::layerName( QgsVectorLayer *vl ) const
{
Q_ASSERT( vl );
if ( mLayerTitleAsName && ( !vl->metadata().title().isEmpty() || !vl->serverProperties()->title().isEmpty() ) )
if ( !mLayerOverriddenName.value( vl->id(), QString() ).isEmpty() )
{
return mLayerOverriddenName.value( vl->id() );
}
else if ( mLayerTitleAsName && ( !vl->metadata().title().isEmpty() || !vl->serverProperties()->title().isEmpty() ) )
{
return !vl->metadata().title().isEmpty() ? vl->metadata().title() : vl->serverProperties()->title();
}
else
return mLayerOverriddenName.value( vl->id(), vl->name() );
{
return vl->name();
}
}

void QgsDxfExport::drawLabel( const QString &layerId, QgsRenderContext &context, pal::LabelPosition *label, const QgsPalLayerSettings &settings )
Expand Down
2 changes: 1 addition & 1 deletion src/core/dxf/qgsdxfexport_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ struct DxfLayerJob
QgsLabelSinkProvider *labelProvider = nullptr;
QgsRuleBasedLabelSinkProvider *ruleBasedLabelProvider = nullptr;
QString splitLayerAttribute;
QString layerDerivedName; // Obtained from layer title, name or overridden name
QString layerDerivedName; // Obtained from overridden name, title or layer name
QSet<QString> attributes;

private:
Expand Down
2 changes: 1 addition & 1 deletion src/ui/qgsdxfexportdialogbase.ui
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@
<item row="0" column="0">
<widget class="QCheckBox" name="mLayerTitleAsName">
<property name="toolTip">
<string>If no attribute is chosen, prefer layer title (set in layer properties) to layer name.</string>
<string>If no attribute is chosen and layer name is not being overridden, prefer layer title (set in layer properties) to layer name.</string>
</property>
<property name="text">
<string>Use layer title as name if set</string>
Expand Down
23 changes: 12 additions & 11 deletions tests/src/core/testqgsdxfexport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1813,8 +1813,8 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence()
{
// Test that output layer name precedence is:
// 1) Attribute (if any)
// 2) Layer title (if any)
// 3) Overridden name (if any)
// 2) Overridden name (if any)
// 3) Layer title (if any)
// 4) Layer name

const QString layerTitle = QStringLiteral( "Point Layer Title" );
Expand Down Expand Up @@ -1871,8 +1871,8 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence()
dxfFile2.close();

QVERIFY( !fileContainsText( file2, QStringLiteral( "nan.0" ) ) );
QVERIFY( fileContainsText( file2, layerTitle ) );
QVERIFY( !fileContainsText( file2, layerOverriddenName ) );
QVERIFY( !fileContainsText( file2, layerTitle ) );
QVERIFY( fileContainsText( file2, layerOverriddenName ) );
QVERIFY( !fileContainsText( file2, mPointLayer->name() ) );

// reload and compare
Expand All @@ -1882,20 +1882,20 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence()
QCOMPARE( result->wkbType(), Qgis::WkbType::Point );
QgsFeature feature;
result->getFeatures().nextFeature( feature );
QCOMPARE( feature.attribute( "Layer" ), layerTitle );
QCOMPARE( feature.attribute( "Layer" ), layerOverriddenName );
QCOMPARE( result->uniqueValues( 0 ).count(), 1 ); // "Layer" field

// C) No attribute given, choose no title
d.setLayerTitleAsName( false );
// C) No attribute given, no override
d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer, -1, false, -1 ) ); // this replaces layers

const QString file3 = getTempFileName( "name_precedence_c_no_attr_no_title_dxf" );
const QString file3 = getTempFileName( "name_precedence_c_no_attr_no_override_dxf" );
QFile dxfFile3( file3 );
QCOMPARE( d.writeToFile( &dxfFile3, QStringLiteral( "CP1252" ) ), QgsDxfExport::ExportResult::Success );
dxfFile3.close();

QVERIFY( !fileContainsText( file3, QStringLiteral( "nan.0" ) ) );
QVERIFY( !fileContainsText( file3, layerTitle ) );
QVERIFY( fileContainsText( file3, layerOverriddenName ) );
QVERIFY( fileContainsText( file3, layerTitle ) );
QVERIFY( !fileContainsText( file3, layerOverriddenName ) );
QVERIFY( !fileContainsText( file3, mPointLayer->name() ) );

// reload and compare
Expand All @@ -1904,11 +1904,12 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence()
QCOMPARE( result->featureCount(), mPointLayer->featureCount() );
QCOMPARE( result->wkbType(), Qgis::WkbType::Point );
result->getFeatures().nextFeature( feature );
QCOMPARE( feature.attribute( "Layer" ), layerOverriddenName );
QCOMPARE( feature.attribute( "Layer" ), layerTitle );
QCOMPARE( result->uniqueValues( 0 ).count(), 1 ); // "Layer" field

// D) No name options given, use default layer name
d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer ) ); // This replaces layers
d.setLayerTitleAsName( false );

const QString file4 = getTempFileName( "name_precedence_d_no_anything_dxf" );
QFile dxfFile4( file4 );
Expand Down

0 comments on commit 82fb6cc

Please sign in to comment.