From 692d05ba236354b214be81ed34ff10458348181e Mon Sep 17 00:00:00 2001 From: nirvn Date: Thu, 29 Aug 2019 16:47:32 +0700 Subject: [PATCH] [clipboard] Fix copying of string attributes containing new lines and tabs characters --- src/app/qgsclipboard.cpp | 66 +++++++++++++------------- src/app/qgsclipboard.h | 2 +- tests/src/app/testqgisappclipboard.cpp | 13 ++--- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/app/qgsclipboard.cpp b/src/app/qgsclipboard.cpp index 65c71416141..6138b4eed00 100644 --- a/src/app/qgsclipboard.cpp +++ b/src/app/qgsclipboard.cpp @@ -75,31 +75,38 @@ void QgsClipboard::replaceWithCopyOf( QgsFeatureStore &featureStore ) emit changed(); } -QString QgsClipboard::generateClipboardText() const +void QgsClipboard::generateClipboardText( QString &textContent, QString &htmlContent ) const { CopyFormat format = QgsSettings().enumValue( QStringLiteral( "qgis/copyFeatureFormat" ), AttributesWithWKT ); + textContent.clear(); + htmlContent.clear(); + switch ( format ) { case AttributesOnly: case AttributesWithWKT: { - QStringList textLines; - QStringList textFields; + QStringList textLines, htmlLines; + QStringList textFields, htmlFields; // first do the field names if ( format == AttributesWithWKT ) { textFields += QStringLiteral( "wkt_geom" ); + htmlFields += QStringLiteral( "wkt_geom" ); } const auto constMFeatureFields = mFeatureFields; for ( const QgsField &field : constMFeatureFields ) { textFields += field.name(); + htmlFields += QStringLiteral( "%1" ).arg( field.name() ); } textLines += textFields.join( QStringLiteral( "\t" ) ); + htmlLines += htmlFields.join( QString() ); textFields.clear(); + htmlFields.clear(); // then the field contents for ( QgsFeatureList::const_iterator it = mFeatureClipboard.constBegin(); it != mFeatureClipboard.constEnd(); ++it ) @@ -110,38 +117,50 @@ QString QgsClipboard::generateClipboardText() const if ( format == AttributesWithWKT ) { if ( it->hasGeometry() ) - textFields += it->geometry().asWkt(); + { + QString wkt = it->geometry().asWkt(); + textFields += wkt; + htmlFields += QStringLiteral( "%1" ).arg( wkt ); + } else { textFields += QgsApplication::nullRepresentation(); + htmlFields += QStringLiteral( "%1" ).arg( QgsApplication::nullRepresentation() ); } } for ( int idx = 0; idx < attributes.count(); ++idx ) { - // QgsDebugMsg(QString("inspecting field '%1'.").arg(it2->toString())); - if ( attributes.at( idx ).toString().contains( QStringLiteral( "\n" ), Qt::CaseInsensitive ) ) - textFields += QStringLiteral( "\"" ) + attributes.at( idx ).toString() + QStringLiteral( "\"" ); + QString value = attributes.at( idx ).toString(); + if ( value.contains( '\n' ) || value.contains( '\t' ) ) + textFields += '"' + value.replace( '"', QStringLiteral( "\"\"" ) ) + '\"'; else { - textFields += attributes.at( idx ).toString(); + textFields += value; } + value = attributes.at( idx ).toString(); + value.replace( '\n', QStringLiteral( "
" ) ).replace( '\t', QStringLiteral( " " ) ); + htmlFields += QStringLiteral( "%1" ).arg( value ); } textLines += textFields.join( QStringLiteral( "\t" ) ); + htmlLines += htmlFields.join( QString() ); textFields.clear(); + htmlFields.clear(); } - return textLines.join( QStringLiteral( "\n" ) ); + textContent = textLines.join( '\n' ); + htmlContent = QStringLiteral( "" ) + htmlLines.join( QStringLiteral( "" ) ) + QStringLiteral( "
" ); + break; } case GeoJSON: { QgsJsonExporter exporter; exporter.setSourceCrs( mCRS ); - return exporter.exportFeatures( mFeatureClipboard ); + + textContent = exporter.exportFeatures( mFeatureClipboard ); } } - return QString(); } void QgsClipboard::setSystemClipboard() @@ -155,31 +174,14 @@ void QgsClipboard::setSystemClipboard() QClipboard *cb = QApplication::clipboard(); // Copy text into the clipboard - QString textCopy = generateClipboardText(); + QString textCopy, htmlCopy; + generateClipboardText( textCopy, htmlCopy ); QMimeData *m = new QMimeData(); m->setText( textCopy ); - if ( mFeatureClipboard.count() < 1000 ) + if ( mFeatureClipboard.count() < 1000 && !htmlCopy.isEmpty() ) { - CopyFormat format = QgsSettings().enumValue( QStringLiteral( "qgis/copyFeatureFormat" ), AttributesWithWKT ); - - QString htmlCopy; - switch ( format ) - { - case AttributesOnly: - case AttributesWithWKT: - htmlCopy = textCopy; - htmlCopy.replace( '\n', QStringLiteral( "" ) ); - htmlCopy.replace( '\t', QStringLiteral( "" ) ); - htmlCopy = QStringLiteral( "
" ) + htmlCopy + QStringLiteral( "
" ); - break; - case GeoJSON: - break; - } - if ( !htmlCopy.isEmpty() ) - { - m->setHtml( htmlCopy ); - } + m->setHtml( htmlCopy ); } // With qgis running under Linux, but with a Windows based X diff --git a/src/app/qgsclipboard.h b/src/app/qgsclipboard.h index cd12265a3ab..48aa68cd2e1 100644 --- a/src/app/qgsclipboard.h +++ b/src/app/qgsclipboard.h @@ -154,7 +154,7 @@ class APP_EXPORT QgsClipboard : public QObject * Creates a text representation of the clipboard features. * \returns clipboard text, respecting user export format */ - QString generateClipboardText() const; + void generateClipboardText( QString &textContent, QString &htmlContent ) const; /** * Attempts to convert a string to a list of features, by parsing the string as WKT and GeoJSON diff --git a/tests/src/app/testqgisappclipboard.cpp b/tests/src/app/testqgisappclipboard.cpp index 554daf5635d..8b9eccdb98f 100644 --- a/tests/src/app/testqgisappclipboard.cpp +++ b/tests/src/app/testqgisappclipboard.cpp @@ -141,12 +141,13 @@ void TestQgisAppClipboard::copyToText() // attributes only QgsSettings settings; settings.setEnumValue( QStringLiteral( "/qgis/copyFeatureFormat" ), QgsClipboard::AttributesOnly ); - QString result = mQgisApp->clipboard()->generateClipboardText(); + QString result, resultHtml; + mQgisApp->clipboard()->generateClipboardText( result, resultHtml ); QCOMPARE( result, QString( "int_field\tstring_field\n9\tval\n19\tval2" ) ); // attributes with WKT settings.setEnumValue( QStringLiteral( "/qgis/copyFeatureFormat" ), QgsClipboard::AttributesWithWKT ); - result = mQgisApp->clipboard()->generateClipboardText(); + mQgisApp->clipboard()->generateClipboardText( result, resultHtml ); QCOMPARE( result, QString( "wkt_geom\tint_field\tstring_field\nPoint (5 6)\t9\tval\nPoint (7 8)\t19\tval2" ) ); // HTML test @@ -156,7 +157,7 @@ void TestQgisAppClipboard::copyToText() // GeoJSON settings.setEnumValue( QStringLiteral( "/qgis/copyFeatureFormat" ), QgsClipboard::GeoJSON ); - result = mQgisApp->clipboard()->generateClipboardText(); + mQgisApp->clipboard()->generateClipboardText( result, resultHtml ); QString expected = "{\"features\":[{\"geometry\":{\"coordinates\":[5.0,6.0],\"type\":\"Point\"},\"id\":5," "\"properties\":{\"int_field\":9,\"string_field\":\"val\"},\"type\":\"Feature\"}," "{\"geometry\":{\"coordinates\":[7.0,8.0],\"type\":\"Point\"},\"id\":6," @@ -174,7 +175,7 @@ void TestQgisAppClipboard::copyToText() feats.setFields( fields ); mQgisApp->clipboard()->replaceWithCopyOf( feats ); - result = mQgisApp->clipboard()->generateClipboardText(); + mQgisApp->clipboard()->generateClipboardText( result, resultHtml ); // just test coordinates as integers - that's enough to verify that reprojection has occurred // and helps avoid rounding issues @@ -211,13 +212,13 @@ void TestQgisAppClipboard::copyToText() // attributes only settings.setEnumValue( QStringLiteral( "/qgis/copyFeatureFormat" ), QgsClipboard::AttributesOnly ); - result = mQgisApp->clipboard()->generateClipboardText(); + mQgisApp->clipboard()->generateClipboardText( result, resultHtml ); qDebug() << result; QCOMPARE( result, QString( "int_field\tstring_field\n1\tSingle line text\n2\t\"Unix Multiline \nText\"\n3\t\"Windows Multiline \r\nText\"" ) ); // attributes with WKT settings.setEnumValue( QStringLiteral( "/qgis/copyFeatureFormat" ), QgsClipboard::AttributesWithWKT ); - result = mQgisApp->clipboard()->generateClipboardText(); + mQgisApp->clipboard()->generateClipboardText( result, resultHtml ); QCOMPARE( result, QString( "wkt_geom\tint_field\tstring_field\nPoint (5 6)\t1\tSingle line text\nPoint (7 8)\t2\t\"Unix Multiline \nText\"\nPoint (9 10)\t3\t\"Windows Multiline \r\nText\"" ) ); }