QCommandLineParser: Wrap very long option names to leave room for des…

…criptions Fixes: QTBUG-79926 Change-Id: I3302e0ed5b58949a35ccb001c71b22a6400a6c81 Reviewed-by: Thiago Macieira <[email protected]>
chain78 · May 1, 2020 · f0ea852 · f0ea852
1 parent a426326
commit f0ea852
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 10 deletions.
diff --git a/src/corelib/tools/qcommandlineparser.cpp b/src/corelib/tools/qcommandlineparser.cpp
@@ -1063,18 +1063,23 @@ QString QCommandLineParser::helpText() const
     return d->helpText(false);
 }
 
-static QString wrapText(const QString &names, int longestOptionNameString, const QString &description)
+static QString wrapText(const QString &names, int optionNameMaxWidth, const QString &description)
 {
     const QLatin1Char nl('\n');
     const QLatin1String indentation("  ");
-    if (description.isEmpty())
-        return indentation + names + nl;
 
-    QString text = indentation + names.leftJustified(longestOptionNameString) + QLatin1Char(' ');
-    const int indent = text.length();
+    // In case the list of option names is very long, wrap it as well
+    int nameIndex = 0;
+    auto nextNameSection = [&]() {
+        QString section = names.mid(nameIndex, optionNameMaxWidth);
+        nameIndex += section.size();
+        return section;
+    };
+
+    QString text;
     int lineStart = 0;
     int lastBreakable = -1;
-    const int max = 79 - indent;
+    const int max = 79 - (indentation.size() + optionNameMaxWidth + 1);
     int x = 0;
     const int len = description.length();
 
@@ -1103,8 +1108,7 @@ static QString wrapText(const QString &names, int longestOptionNameString, const
         if (breakAt != -1) {
             const int numChars = breakAt - lineStart;
             //qDebug() << "breakAt=" << description.at(breakAt) << "breakAtSpace=" << breakAtSpace << lineStart << "to" << breakAt << description.mid(lineStart, numChars);
-            if (lineStart > 0)
-                text += QString(indent, QLatin1Char(' '));
+            text += indentation + nextNameSection().leftJustified(optionNameMaxWidth) + QLatin1Char(' ');
             text += description.midRef(lineStart, numChars) + nl;
             x = 0;
             lastBreakable = -1;
@@ -1115,6 +1119,10 @@ static QString wrapText(const QString &names, int longestOptionNameString, const
         }
     }
 
+    while (nameIndex < names.size()) {
+        text += indentation + nextNameSection() + nl;
+    }
+
     return text;
 }
 
@@ -1158,19 +1166,20 @@ QString QCommandLineParserPrivate::helpText(bool includeQtOptions) const
         longestOptionNameString = qMax(longestOptionNameString, optionNamesString.length());
     }
     ++longestOptionNameString;
+    const int optionNameMaxWidth = qMin(50, longestOptionNameString);
     auto optionNameIterator = optionNameList.cbegin();
     for (const QCommandLineOption &option : qAsConst(options)) {
         if (option.flags() & QCommandLineOption::HiddenFromHelp)
             continue;
-        text += wrapText(*optionNameIterator, longestOptionNameString, option.description());
+        text += wrapText(*optionNameIterator, optionNameMaxWidth, option.description());
         ++optionNameIterator;
     }
     if (!positionalArgumentDefinitions.isEmpty()) {
         if (!options.isEmpty())
             text += nl;
         text += QCommandLineParser::tr("Arguments:") + nl;
         for (const PositionalArgumentDefinition &arg : positionalArgumentDefinitions)
-            text += wrapText(arg.name, longestOptionNameString, arg.description);
+            text += wrapText(arg.name, optionNameMaxWidth, arg.description);
     }
     return text;
 }

diff --git a/tests/auto/corelib/tools/qcommandlineparser/testhelper/qcommandlineparser_test_helper.cpp b/tests/auto/corelib/tools/qcommandlineparser/testhelper/qcommandlineparser_test_helper.cpp
@@ -97,6 +97,13 @@ int main(int argc, char *argv[])
         parser.process(app);
         const QString size = parser.value("size");
         printf("Resizing %s to %s and saving to %s\n", qPrintable(parser.value("load")), qPrintable(size), qPrintable(parser.value("o")));
+    } else if (command == "long") {
+        // A very long option (QTBUG-79926)
+        QCommandLineOption longOption(QStringList{QStringLiteral("looooooooooooong-option"), QStringLiteral("looooong-opt-alias")});
+        longOption.setDescription(QStringLiteral("Short description"));
+        longOption.setValueName(QStringLiteral("looooooooooooong-value-name"));
+        parser.addOption(longOption);
+        parser.process(app);
     } else {
         // Call process again, to handle unknown options this time.
         parser.process(app);

diff --git a/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp b/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp
@@ -78,6 +78,7 @@ private slots:
     void testUnknownOption();
     void testHelpAll_data();
     void testHelpAll();
+    void testVeryLongOptionNames();
 };
 
 static char *empty_argv[] = { 0 };
@@ -737,6 +738,38 @@ void tst_QCommandLineParser::testHelpAll()
 #endif // QT_CONFIG(process)
 }
 
+void tst_QCommandLineParser::testVeryLongOptionNames()
+{
+#if !QT_CONFIG(process)
+    QSKIP("This test requires QProcess support");
+#else
+#if defined(Q_OS_ANDROID) && !defined(Q_OS_ANDROID_EMBEDDED)
+    QSKIP("Deploying executable applications to file system on Android not supported.");
+#endif
+
+    QCoreApplication app(empty_argc, empty_argv);
+    QProcess process;
+    process.start("testhelper/qcommandlineparser_test_helper", QStringList() << "0" << "long" << "--help");
+    QVERIFY(process.waitForFinished(5000));
+    QCOMPARE(process.exitStatus(), QProcess::NormalExit);
+    QString output = process.readAll();
+#ifdef Q_OS_WIN
+    output.replace(QStringLiteral("\r\n"), QStringLiteral("\n"));
+#endif
+    const QStringList lines = output.split('\n');
+    const int last = lines.count() - 1;
+    // Let's not compare everything, just the final parts.
+    QCOMPARE(lines.at(last - 7), "                                                     cdefghijklmnopqrstuvwxyz");
+    QCOMPARE(lines.at(last - 6), "  --looooooooooooong-option, --looooong-opt-alias <l Short description");
+    QCOMPARE(lines.at(last - 5), "  ooooooooooooong-value-name>");
+    QCOMPARE(lines.at(last - 4), "");
+    QCOMPARE(lines.at(last - 3), "Arguments:");
+    QCOMPARE(lines.at(last - 2), "  parsingMode                                        The parsing mode to test.");
+    QCOMPARE(lines.at(last - 1), "  command                                            The command to execute.");
+
+#endif // QT_CONFIG(process)
+}
+
 QTEST_APPLESS_MAIN(tst_QCommandLineParser)
 #include "tst_qcommandlineparser.moc"