Merge branch 'issue-1613' into calamares

The popup now cuts down messages to a manageable length.
Hopefully the part that is preserved, will still show
something meaningful for the user (8 lines of text should
be sufficient for the kind of things we do).

FIXES #1613

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,8 +175,53 @@ 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 );
+    }
 
-    return front + back;
+    if ( front.length() + back.length() <= chars.total )
+    {
+        return front + back;
+    }
+
+    // We need to cut off some bits, preserving whether there are
+    // newlines present at the end of the string. Go case-by-case:
+    if ( !front.isEmpty() && back.isEmpty() )
+    {
+        // Truncate towards the front
+        bool needsNewline = front.endsWith( NEWLINE );
+        front.truncate( chars.total );
+        if ( !front.endsWith( NEWLINE ) && needsNewline )
+        {
+            front.append( NEWLINE );
+        }
+        return front;
+    }
+    if ( front.isEmpty() && !back.isEmpty() )
+    {
+        // Truncate towards the tail
+        return back.right( chars.total );
+    }
+    // Both are non-empty, so nibble away at both of them
+    front.truncate( chars.total / 2 );
+    if ( !front.endsWith( NEWLINE ) && physicalLinesInString > 0 )
+    {
+        front.append( NEWLINE );
+    }
+    return front + back.right( chars.total / 2 );
 }
 
 

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.

src/libcalamares/utils/Tests.cpp

@@ -67,6 +67,7 @@ private Q_SLOTS:
     /** @brief Test smart string truncation. */
     void testStringTruncation();
     void testStringTruncationShorter();
+    void testStringTruncationDegenerate();
 
 private:
     void recursiveCompareMap( const QVariantMap& a, const QVariantMap& b, int depth );
@@ -597,6 +598,8 @@ and the translations updated.)" );
     QVERIFY( !longString.endsWith( NEWLINE ) );
     QCOMPARE( longString.count( NEWLINE ), 2 );
     QVERIFY( longString.length() > insufficientLength );
+    // Even the first line must be more than the insufficientLength
+    QVERIFY( longString.indexOf( NEWLINE ) > insufficientLength );
 
     // Grab first line, untruncated
     {
@@ -626,11 +629,84 @@ and the translations updated.)" );
         QVERIFY( longString.endsWith( s ) );
         QVERIFY( !s.endsWith( NEWLINE ) );
         QVERIFY( s.endsWith( "updated." ) );
-        cDebug() << "Result-line" << Logger::Quote << s;
         QCOMPARE( s.count( NEWLINE ), 1 );  // Because last line doesn't end with a newline
         QVERIFY( s.startsWith( "displayed in " ) );
     }
 
+    // First line, truncated
+    {
+        auto s = truncateMultiLine( longString, LinesStartEnd { 1, 0 }, CharCount { insufficientLength } );
+        cDebug() << "Result-line" << Logger::Quote << s;
+        QVERIFY( s.length() > 1 );
+        QVERIFY( s.endsWith( NEWLINE ) );
+        QVERIFY( s.startsWith( "Some " ) );
+        // Because the first line has a newline, the truncated version does too,
+        //   but that makes it one longer than requested.
+        QCOMPARE( s.length(), insufficientLength + 1 );
+        QVERIFY( longString.startsWith( s.left( insufficientLength ) ) );
+    }
+
+    // Last line, truncated; this line is quite short
+    {
+        const int quiteShort = 8;
+        QVERIFY( longString.lastIndexOf( NEWLINE ) < longString.length() - quiteShort );
+
+        auto s = truncateMultiLine( longString, LinesStartEnd { 0, 1 }, CharCount { quiteShort } );
+        cDebug() << "Result-line" << Logger::Quote << s;
+        QVERIFY( s.length() > 1 );
+        QVERIFY( !s.endsWith( NEWLINE ) );  // Because the original doesn't either
+        QVERIFY( s.startsWith( "upda" ) );
+        QCOMPARE( s.length(), quiteShort );  // No extra newlines
+        QVERIFY( longString.endsWith( s ) );
+    }
+
+    // First and last, but both truncated
+    {
+        const int quiteShort = 16;
+        QVERIFY( longString.indexOf( NEWLINE ) > quiteShort );
+        QVERIFY( longString.lastIndexOf( NEWLINE ) < longString.length() - quiteShort );
+
+        auto s = truncateMultiLine( longString, LinesStartEnd { 1, 1 }, CharCount { quiteShort } );
+        cDebug() << "Result-line" << Logger::Quote << s;
+        QVERIFY( s.length() > 1 );
+        QVERIFY( !s.endsWith( NEWLINE ) );  // Because the original doesn't either
+        QVERIFY( s.startsWith( "Some " ) );
+        QVERIFY( s.endsWith( "updated." ) );
+        QCOMPARE( s.length(), quiteShort + 1 );  // Newline between front and back part
+    }
+}
+
+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 char NEWLINE = '\n';
+    const int quiteShort = 16;
+    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 );  // 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 );
+    }
 }