From eafb8149b356fad37c919955f6f50515b456bd17 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 2 Feb 2021 15:35:53 +0100
Subject: [PATCH] [libcalamares] Test some degenerate truncation cases

---
 src/libcalamares/utils/String.cpp | 25 +++++++++++++++++++++----
 src/libcalamares/utils/String.h   |  2 ++
 src/libcalamares/utils/Tests.cpp  | 29 ++++++++++++++++-------------
 3 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/src/libcalamares/utils/String.cpp b/src/libcalamares/utils/String.cpp
index 02b41f1c0..615d30309 100644
--- a/src/libcalamares/utils/String.cpp
+++ b/src/libcalamares/utils/String.cpp
@@ -135,14 +135,15 @@ truncateMultiLine( const QString& string, CalamaresUtils::LinesStartEnd lines, C
         return shorter;
     }
 
-    const int linesInString = string.count( NEWLINE ) + ( string.endsWith( NEWLINE ) ? 0 : 1 );
-    if ( ( string.length() <= chars.total ) && ( linesInString <= maxLines ) )
+    const int physicalLinesInString = string.count( NEWLINE );
+    const int logicalLinesInString = physicalLinesInString + ( string.endsWith( NEWLINE ) ? 0 : 1 );
+    if ( ( string.length() <= chars.total ) && ( logicalLinesInString <= maxLines ) )
     {
         return string;
     }
 
     QString front, back;
-    if ( string.count( NEWLINE ) >= maxLines )
+    if ( physicalLinesInString >= maxLines )
     {
         int from = -1;
         for ( int i = 0; i < lines.atStart; ++i )
@@ -174,6 +175,22 @@ truncateMultiLine( const QString& string, CalamaresUtils::LinesStartEnd lines, C
             back = string.right( lastNewLine );
         }
     }
+    else
+    {
+        // We have: <= maxLines and longer than chars.total, so:
+        //   - carve out a chunk in the middle, based a little on
+        //     how the balance of atStart and atEnd is
+        const int charsToChop = string.length() - chars.total;
+        if ( charsToChop < 1 )
+        {
+            // That's strange, again
+            return string;
+        }
+        const int startPortion = charsToChop * lines.atStart / maxLines;
+        const int endPortion = charsToChop * lines.atEnd / maxLines;
+        front = string.left( string.length() / 2 - startPortion );
+        back = string.right( string.length() / 2 - endPortion );
+    }
 
     if ( front.length() + back.length() <= chars.total )
     {
@@ -200,7 +217,7 @@ truncateMultiLine( const QString& string, CalamaresUtils::LinesStartEnd lines, C
     }
     // Both are non-empty, so nibble away at both of them
     front.truncate( chars.total / 2 );
-    if ( !front.endsWith( NEWLINE ) )
+    if ( !front.endsWith( NEWLINE ) && physicalLinesInString > 0 )
     {
         front.append( NEWLINE );
     }
diff --git a/src/libcalamares/utils/String.h b/src/libcalamares/utils/String.h
index 43e0474fa..e08255f86 100644
--- a/src/libcalamares/utils/String.h
+++ b/src/libcalamares/utils/String.h
@@ -89,6 +89,8 @@ struct CharCount
  * @p lines.atStart is zero) or end (if @p lines.atEnd is zero) or in the middle
  * (if both are nonzero).
  *
+ * Asking for 0 lines will make this behave like QString::truncate().
+ *
  * @param string the input string.
  * @param lines number of lines to preserve.
  * @param chars maximum number of characters in the returned string.
diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp
index 3ee48890e..cdb37f20d 100644
--- a/src/libcalamares/utils/Tests.cpp
+++ b/src/libcalamares/utils/Tests.cpp
@@ -676,33 +676,36 @@ and the translations updated.)" );
     }
 }
 
-void LibCalamaresTests::testStringTruncationDegenerate()
+void
+LibCalamaresTests::testStringTruncationDegenerate()
 {
     Logger::setupLogLevel( Logger::LOGDEBUG );
 
     using namespace CalamaresUtils;
 
     // This is quite long, 1 line only, with no newlines
-    const QString longString(
-        "The portscout new distfile checker has detected that one or more of your"
-        "ports appears to be out of date. Please take the opportunity to check"
-        "each of the ports listed below, and if possible and appropriate,"
-        "submit/commit an update. If any ports have already been updated, you can"
-        "safely ignore the entry." );
+    const QString longString( "The portscout new distfile checker has detected that one or more of your "
+                              "ports appears to be out of date. Please take the opportunity to check "
+                              "each of the ports listed below, and if possible and appropriate, "
+                              "submit/commit an update. If any ports have already been updated, you can "
+                              "safely ignore the entry." );
 
     const char NEWLINE = '\n';
     const int quiteShort = 16;
-    QVERIFY( longString.length() > quiteShort);
-    QVERIFY( !longString.contains(NEWLINE));
-    QVERIFY( longString.indexOf(NEWLINE) < 0);
+    QVERIFY( longString.length() > quiteShort );
+    QVERIFY( !longString.contains( NEWLINE ) );
+    QVERIFY( longString.indexOf( NEWLINE ) < 0 );
 
     {
         auto s = truncateMultiLine( longString, LinesStartEnd { 1, 0 }, CharCount { quiteShort } );
         cDebug() << "Result-line" << Logger::Quote << s;
         QVERIFY( s.length() > 1 );
-        QCOMPARE( s.length(), quiteShort);  // Newline between front and back part
-        QVERIFY( s.startsWith( "The " ) );
-        QVERIFY( s.endsWith( "entry." ) );
+        QCOMPARE( s.length(), quiteShort );  // No newline between front and back part
+        QVERIFY( s.startsWith( "The port" ) );  // 8, which is quiteShort / 2
+        QVERIFY( s.endsWith( "e entry." ) );  // also 8 chars
+
+        auto t = truncateMultiLine( longString, LinesStartEnd { 2, 2 }, CharCount { quiteShort } );
+        QCOMPARE( s, t );
     }
 }