improve ProWriter editing capabilities

- completely replacing existing value lists
- creating assignments instead of appendings
- writing value lists as a single line

src/plugins/qt4projectmanager/qt-s60/s60publisherovi.cpp

@@ -266,7 +266,8 @@ void S60PublisherOvi::updateProFile(const QString &var, const QString &values)
 
     //todo: after ossi has added scope profile writing, make sure the following works
     //QString scope("symbian");
-    ProWriter::addVarValues(profile, &lines, QStringList() << values, var);
+    ProWriter::putVarValues(profile, &lines, QStringList() << values, var,
+                            ProWriter::ReplaceValues | ProWriter::OneLine | ProWriter::AssignOperator);
 
     if (qfile.open(QIODevice::WriteOnly | QIODevice::Text)) {
         qfile.write(lines.join("\n").toLocal8Bit());

src/shared/proparser/prowriter.cpp

@@ -174,53 +174,86 @@ static const ushort *skipToken(ushort tok, const ushort *&tokPtr, int &lineNo)
     return 0;
 }
 
-void ProWriter::addVarValues(ProFile *profile, QStringList *lines,
+bool ProWriter::locateVarValues(const ushort *tokPtr, const QString &var, int *bestLine)
-    const QStringList &values, const QString &var)
 {
-    // Check if variable item exists as child of root item
-    const ushort *tokPtr = profile->tokPtr();
     int lineNo = 0;
     QString tmp;
     const ushort *lastXpr = 0;
     while (ushort tok = *tokPtr++) {
         if (tok == TokAssign || tok == TokAppend || tok == TokAppendUnique) {
             if (getLiteral(lastXpr, tokPtr - 1, tmp) && var == tmp) {
-                for (--lineNo; lineNo < lines->count(); lineNo++) {
+                *bestLine = lineNo - 1;
-                    QString line = lines->at(lineNo);
+                return true;
-                    int idx = line.indexOf(QLatin1Char('#'));
-                    if (idx >= 0)
-                        line.truncate(idx);
-                    while (line.endsWith(QLatin1Char(' ')) || line.endsWith(QLatin1Char('\t')))
-                        line.chop(1);
-                    if (line.isEmpty()) {
-                        if (idx >= 0)
-                            continue;
-                        break;
-                    }
-                    if (!line.endsWith(QLatin1Char('\\'))) {
-                        (*lines)[lineNo].insert(line.length(), QLatin1String(" \\"));
-                        lineNo++;
-                        break;
-                    }
-                }
-                QString added;
-                foreach (const QString &v, values)
-                    added += QLatin1String("    ") + v + QLatin1String(" \\\n");
-                added.chop(3);
-                lines->insert(lineNo, added);
-                return;
             }
             skipExpression(++tokPtr, lineNo);
         } else {
             lastXpr = skipToken(tok, tokPtr, lineNo);
         }
     }
+    *bestLine = qMax(lineNo - 1, 0);
+    return false;
+}
 
-    // Create & append new variable item
+static int skipContLines(QStringList *lines, int lineNo, bool addCont)
-    QString added = QLatin1Char('\n') + var + QLatin1String(" +=");
+{
-    foreach (const QString &v, values)
+    for (; lineNo < lines->count(); lineNo++) {
-        added += QLatin1String(" \\\n    ") + v;
+        QString line = lines->at(lineNo);
-    *lines << added;
+        int idx = line.indexOf(QLatin1Char('#'));
+        if (idx >= 0)
+            line.truncate(idx);
+        while (line.endsWith(QLatin1Char(' ')) || line.endsWith(QLatin1Char('\t')))
+            line.chop(1);
+        if (line.isEmpty()) {
+            if (idx >= 0)
+                continue;
+            break;
+        }
+        if (!line.endsWith(QLatin1Char('\\'))) {
+            if (addCont)
+                (*lines)[lineNo].insert(line.length(), QLatin1String(" \\"));
+            lineNo++;
+            break;
+        }
+    }
+    return lineNo;
+}
+
+void ProWriter::putVarValues(ProFile *profile, QStringList *lines,
+    const QStringList &values, const QString &var, PutFlags flags)
+{
+    int lineNo;
+    if (locateVarValues(profile->tokPtr(), var, &lineNo)) {
+        if (flags & ReplaceValues) {
+            // remove continuation lines with old values
+            int lNo = skipContLines(lines, lineNo, false);
+            lines->erase(lines->begin() + lineNo + 1, lines->begin() + lNo);
+            // remove rest of the line
+            QString &line = (*lines)[lineNo];
+            int eqs = line.indexOf(QLatin1Char('='));
+            if (eqs >= 0) // If this is not true, we mess up the file a bit.
+                line.truncate(eqs + 1);
+            // put new values
+            foreach (const QString &v, values)
+                line += ((flags & MultiLine) ? QLatin1String(" \\\n    ") : QLatin1String(" ")) + v;
+        } else {
+            lineNo = skipContLines(lines, lineNo, true);
+            QString added;
+            foreach (const QString &v, values)
+                added += QLatin1String("    ") + v + QLatin1String(" \\\n");
+            added.chop(3);
+            lines->insert(lineNo, added);
+        }
+    } else {
+        // Create & append new variable item
+        QString added;
+        int lNo = skipContLines(lines, lineNo, false);
+        if (lNo)
+            added += QLatin1Char('\n');
+        added += var + QLatin1String((flags & AppendOperator) ? " +=" : " =");
+        foreach (const QString &v, values)
+            added += ((flags & MultiLine) ? QLatin1String(" \\\n    ") : QLatin1String(" ")) + v;
+        lines->insert(lNo, added);
+    }
 }
 
 void ProWriter::addFiles(ProFile *profile, QStringList *lines,
@@ -230,7 +263,7 @@ void ProWriter::addFiles(ProFile *profile, QStringList *lines,
     foreach (const QString &v, values)
         valuesToWrite << proFileDir.relativeFilePath(v);
 
-    addVarValues(profile, lines, valuesToWrite, var);
+    putVarValues(profile, lines, valuesToWrite, var, AppendValues | MultiLine | AppendOperator);
 }
 
 static void findProVariables(const ushort *tokPtr, const QStringList &vars,

src/shared/proparser/prowriter.h

@@ -49,8 +49,18 @@ namespace Internal {
 class ProWriter
 {
 public:
-    static void addVarValues(ProFile *profile, QStringList *lines,
+    enum PutFlag {
-        const QStringList &values, const QString &var);
+        AppendValues = 0,
+        ReplaceValues = 1,
+        OneLine = 0, // this works only when replacing (or adding a new assignment)
+        MultiLine = 2,
+        AssignOperator = 0, // ignored when changing an existing assignment
+        AppendOperator = 4
+    };
+    Q_DECLARE_FLAGS(PutFlags, PutFlag)
+
+    static void putVarValues(ProFile *profile, QStringList *lines,
+        const QStringList &values, const QString &var, PutFlags flags);
     static QList<int> removeVarValues(ProFile *profile, QStringList *lines,
         const QStringList &values, const QStringList &vars);
 
@@ -58,8 +68,13 @@ public:
          const QDir &proFileDir, const QStringList &filePaths, const QString &var);
     static QStringList removeFiles(ProFile *profile, QStringList *lines,
         const QDir &proFileDir, const QStringList &filePaths, const QStringList &vars);
+
+private:
+    static bool locateVarValues(const ushort *tokPtr, const QString &var, int *bestLine);
 };
 
+Q_DECLARE_OPERATORS_FOR_FLAGS(ProWriter::PutFlags)
+
 } // namespace Internal
 } // namespace Qt4ProjectManager
 

tests/auto/profilewriter/tst_profilewriter.cpp

@@ -58,27 +58,41 @@ static ParseHandler parseHandler;
 
 //////////////// the actual autotest
 
+typedef Qt4ProjectManager::Internal::ProWriter PW;
+
 class tst_ProFileWriter : public QObject
 {
     Q_OBJECT
 
 private slots:
-    void edit_data();
+    void adds_data();
-    void edit();
+    void adds();
+    void removes_data();
+    void removes();
     void multiVar();
+    void addFiles();
+    void removeFiles();
 };
 
-void tst_ProFileWriter::edit_data()
+static QStringList strList(const char * const *array)
 {
-    QTest::addColumn<bool>("add");
+    QStringList values;
-    QTest::addColumn<QStringList>("files");
+    for (const char * const *value = array; *value; value++)
+        values << QString::fromLatin1(*value);
+    return values;
+}
+
+void tst_ProFileWriter::adds_data()
+{
+    QTest::addColumn<int>("flags");
+    QTest::addColumn<QStringList>("values");
     QTest::addColumn<QString>("input");
     QTest::addColumn<QString>("output");
 
     struct Case {
-        bool add;
+        int flags;
         const char *title;
-        const char * const *files;
+        const char * const *values;
         const char *input;
         const char *output;
     };
@@ -86,9 +100,16 @@ void tst_ProFileWriter::edit_data()
     static const char *f_foo[] = { "foo", 0 };
     static const char *f_foo_bar[] = { "foo", "bar", 0 };
     static const Case cases[] = {
-        // Adding entries
         {
-            true, "add new", f_foo,
+            PW::AppendValues|PW::AppendOperator|PW::MultiLine,
+            "add new append multi", f_foo,
+            "",
+            "SOURCES += \\\n"
+            "    foo"
+        },
+        {
+            PW::AppendValues|PW::AppendOperator|PW::MultiLine,
+            "add new append multi after comment", f_foo,
             "# test file",
             "# test file\n"
             "\n"
@@ -96,7 +117,72 @@ void tst_ProFileWriter::edit_data()
             "    foo"
         },
         {
-            true, "add new ignoring scoped", f_foo,
+            PW::AppendValues|PW::AppendOperator|PW::MultiLine,
+            "add new append multi before newlines", f_foo,
+            "\n"
+            "\n"
+            "\n",
+            "SOURCES += \\\n"
+            "    foo\n"
+            "\n"
+            "\n"
+            "\n"
+        },
+        {
+            PW::AppendValues|PW::AppendOperator|PW::MultiLine,
+            "add new append multi after comment before newlines", f_foo,
+            "# test file\n"
+            "\n"
+            "\n"
+            "\n",
+            "# test file\n"
+            "\n"
+            "SOURCES += \\\n"
+            "    foo\n"
+            "\n"
+            "\n"
+            "\n"
+        },
+        {
+            PW::AppendValues|PW::AssignOperator|PW::MultiLine,
+            "add new assign multi", f_foo,
+            "# test file",
+            "# test file\n"
+            "\n"
+            "SOURCES = \\\n"
+            "    foo"
+        },
+        {
+            PW::AppendValues|PW::AppendOperator|PW::OneLine,
+            "add new append oneline", f_foo,
+            "# test file",
+            "# test file\n"
+            "\n"
+            "SOURCES += foo"
+        },
+        {
+            PW::AppendValues|PW::AssignOperator|PW::OneLine,
+            "add new assign oneline", f_foo,
+            "# test file",
+            "# test file\n"
+            "\n"
+            "SOURCES = foo"
+        },
+        {
+            PW::AppendValues|PW::AssignOperator|PW::OneLine,
+            "add new assign oneline after existing", f_foo,
+            "# test file\n"
+            "\n"
+            "HEADERS = foo",
+            "# test file\n"
+            "\n"
+            "HEADERS = foo\n"
+            "\n"
+            "SOURCES = foo"
+        },
+        {
+            PW::AppendValues|PW::AppendOperator|PW::MultiLine,
+            "add new ignoring scoped", f_foo,
             "unix:SOURCES = some files",
             "unix:SOURCES = some files\n"
             "\n"
@@ -104,19 +190,22 @@ void tst_ProFileWriter::edit_data()
             "    foo"
         },
         {
-            true, "add to existing", f_foo,
+            PW::AppendValues|PW::AppendOperator|PW::MultiLine,
+            "add to existing", f_foo,
             "SOURCES = some files",
             "SOURCES = some files \\\n"
             "    foo"
         },
         {
-            true, "add to existing after comment", f_foo,
+            PW::AppendValues|PW::AppendOperator|PW::MultiLine,
+            "add to existing after comment", f_foo,
             "SOURCES = some files   # comment",
             "SOURCES = some files \\   # comment\n"
             "    foo"
         },
         {
-            true, "add to existing after comment line", f_foo,
+            PW::AppendValues|PW::AppendOperator|PW::MultiLine,
+            "add to existing after comment line", f_foo,
             "SOURCES = some \\\n"
             "   # comment\n"
             "    files",
@@ -125,25 +214,132 @@ void tst_ProFileWriter::edit_data()
             "    files \\\n"
             "    foo"
         },
-
-        // Removing entries
         {
-            false, "remove fail", f_foo,
+            PW::AppendValues|PW::AssignOperator|PW::MultiLine,
+            "add to existing", f_foo,
+            "SOURCES = some files",
+            "SOURCES = some files \\\n"
+            "    foo"
+        },
+        {
+            PW::ReplaceValues|PW::AssignOperator|PW::MultiLine,
+            "replace existing multi", f_foo_bar,
+            "SOURCES = some files",
+            "SOURCES = \\\n"
+            "    foo \\\n"
+            "    bar"
+        },
+        {
+            PW::ReplaceValues|PW::AssignOperator|PW::OneLine,
+            "replace existing oneline", f_foo_bar,
+            "SOURCES = some files",
+            "SOURCES = foo bar"
+        },
+        {
+            PW::ReplaceValues|PW::AssignOperator|PW::OneLine,
+            "replace existing complex last", f_foo_bar,
+            "SOURCES = some \\\n"
+            "   # comment\n"
+            "    files",
+            "SOURCES = foo bar"
+        },
+        {
+            PW::ReplaceValues|PW::AssignOperator|PW::OneLine,
+            "replace existing complex middle 1", f_foo_bar,
+            "SOURCES = some \\\n"
+            "   # comment\n"
+            "    files\n"
+            "HEADERS = blubb",
+            "SOURCES = foo bar\n"
+            "HEADERS = blubb"
+        },
+        {
+            PW::ReplaceValues|PW::AssignOperator|PW::OneLine,
+            "replace existing complex middle 2", f_foo_bar,
+            "SOURCES = some \\\n"
+            "   # comment\n"
+            "    files\n"
+            "\n"
+            "HEADERS = blubb",
+            "SOURCES = foo bar\n"
+            "\n"
+            "HEADERS = blubb"
+        },
+        {
+            PW::ReplaceValues|PW::AssignOperator|PW::OneLine,
+            "replace existing complex middle 3", f_foo_bar,
+            "SOURCES = some \\\n"
+            "   # comment\n"
+            "    files \\\n"
+            "\n"
+            "HEADERS = blubb",
+            "SOURCES = foo bar\n"
+            "\n"
+            "HEADERS = blubb"
+        },
+    };
+
+    for (uint i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
+        const Case *_case = &cases[i];
+        QTest::newRow(_case->title)
+                << _case->flags
+                << strList(_case->values)
+                << QString::fromLatin1(_case->input)
+                << QString::fromLatin1(_case->output);
+    }
+}
+
+void tst_ProFileWriter::adds()
+{
+    QFETCH(int, flags);
+    QFETCH(QStringList, values);
+    QFETCH(QString, input);
+    QFETCH(QString, output);
+
+    QStringList lines = input.isEmpty() ? QStringList() : input.split(QLatin1String("\n"));
+    QString var = QLatin1String("SOURCES");
+
+    ProFileParser parser(0, &parseHandler);
+    ProFile *proFile = parser.parsedProFile(QLatin1String(BASE_DIR "/test.pro"), false, &input);
+    QVERIFY(proFile);
+    PW::putVarValues(proFile, &lines, values, var, PW::PutFlags(flags));
+
+    QCOMPARE(lines.join(QLatin1String("\n")), output);
+}
+
+void tst_ProFileWriter::removes_data()
+{
+    QTest::addColumn<QStringList>("values");
+    QTest::addColumn<QString>("input");
+    QTest::addColumn<QString>("output");
+
+    struct Case {
+        const char *title;
+        const char * const *values;
+        const char *input;
+        const char *output;
+    };
+
+    static const char *f_foo[] = { "foo", 0 };
+    static const char *f_foo_bar[] = { "foo", "bar", 0 };
+    static const Case cases[] = {
+        {
+            "remove fail", f_foo,
             "SOURCES = bak bar",
             "SOURCES = bak bar"
         },
         {
-            false, "remove one-line middle", f_foo,
+            "remove one-line middle", f_foo,
             "SOURCES = bak foo bar",
             "SOURCES = bak bar"
         },
         {
-            false, "remove one-line trailing", f_foo,
+            "remove one-line trailing", f_foo,
             "SOURCES = bak bar foo",
             "SOURCES = bak bar"
         },
         {
-            false, "remove multi-line single leading", f_foo,
+            "remove multi-line single leading", f_foo,
             "SOURCES = foo \\\n"
             "    bak \\\n"
             "    bar",
@@ -152,7 +348,7 @@ void tst_ProFileWriter::edit_data()
             "    bar"
         },
         {
-            false, "remove multi-line single middle", f_foo,
+            "remove multi-line single middle", f_foo,
             "SOURCES = bak \\\n"
             "    foo \\\n"
             "    bar",
@@ -160,7 +356,7 @@ void tst_ProFileWriter::edit_data()
             "    bar"
         },
         {
-            false, "remove multi-line single trailing", f_foo,
+            "remove multi-line single trailing", f_foo,
             "SOURCES = bak \\\n"
             "    bar \\\n"
             "    foo",
@@ -168,7 +364,7 @@ void tst_ProFileWriter::edit_data()
             "    bar"
         },
         {
-            false, "remove multi-line single leading with comment", f_foo,
+            "remove multi-line single leading with comment", f_foo,
             "SOURCES = foo \\  # comment\n"
             "    bak \\\n"
             "    bar",
@@ -177,7 +373,7 @@ void tst_ProFileWriter::edit_data()
             "    bar"
         },
         {
-            false, "remove multi-line single middle with comment", f_foo,
+            "remove multi-line single middle with comment", f_foo,
             "SOURCES = bak \\\n"
             "    foo \\  # comment\n"
             "    bar",
@@ -186,7 +382,7 @@ void tst_ProFileWriter::edit_data()
             "    bar"
         },
         {
-            false, "remove multi-line single trailing with comment", f_foo,
+            "remove multi-line single trailing with comment", f_foo,
             "SOURCES = bak \\\n"
             "    bar \\\n"
             "    foo  # comment",
@@ -195,7 +391,7 @@ void tst_ProFileWriter::edit_data()
             "     # foo # comment"
         },
         {
-            false, "remove multi-line single trailing after empty line", f_foo,
+            "remove multi-line single trailing after empty line", f_foo,
             "SOURCES = bak \\\n"
             "    bar \\\n"
             "    \\\n"
@@ -204,7 +400,7 @@ void tst_ProFileWriter::edit_data()
             "    bar\n"
         },
         {
-            false, "remove multi-line single trailing after comment line", f_foo,
+            "remove multi-line single trailing after comment line", f_foo,
             "SOURCES = bak \\\n"
             "    bar \\\n"
             "       # just a comment\n"
@@ -214,7 +410,7 @@ void tst_ProFileWriter::edit_data()
             "       # just a comment"
         },
         {
-            false, "remove multi-line single trailing after empty line with comment", f_foo,
+            "remove multi-line single trailing after empty line with comment", f_foo,
             "SOURCES = bak \\\n"
             "    bar \\\n"
             "    \\ # just a comment\n"
@@ -224,27 +420,27 @@ void tst_ProFileWriter::edit_data()
             "     # just a comment"
         },
         {
-            false, "remove multiple one-line middle", f_foo_bar,
+            "remove multiple one-line middle", f_foo_bar,
             "SOURCES = bak foo bar baz",
             "SOURCES = bak baz"
         },
         {
-            false, "remove multiple one-line trailing", f_foo_bar,
+            "remove multiple one-line trailing", f_foo_bar,
             "SOURCES = bak baz foo bar",
             "SOURCES = bak baz"
         },
         {
-            false, "remove multiple one-line interleaved", f_foo_bar,
+            "remove multiple one-line interleaved", f_foo_bar,
             "SOURCES = bak foo baz bar",
             "SOURCES = bak baz"
         },
         {
-            false, "remove multiple one-line middle with comment", f_foo_bar,
+            "remove multiple one-line middle with comment", f_foo_bar,
             "SOURCES = bak foo bar baz   # comment",
             "SOURCES = bak baz   # bar # foo # comment"
         },
         {
-            false, "remove multi-line multiple trailing with empty line with comment", f_foo_bar,
+            "remove multi-line multiple trailing with empty line with comment", f_foo_bar,
             "SOURCES = bak \\\n"
             "    bar \\\n"
             "    \\ # just a comment\n"
@@ -256,35 +452,26 @@ void tst_ProFileWriter::edit_data()
 
     for (uint i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
         const Case *_case = &cases[i];
-        QStringList files;
-        for (const char * const *file = _case->files; *file; file++)
-            files << QString::fromLatin1(BASE_DIR "/") + QString::fromLatin1(*file);
         QTest::newRow(_case->title)
-                << _case->add
+                << strList(_case->values)
-                << files
                 << QString::fromLatin1(_case->input)
                 << QString::fromLatin1(_case->output);
     }
 }
 
-void tst_ProFileWriter::edit()
+void tst_ProFileWriter::removes()
 {
-    QFETCH(bool, add);
+    QFETCH(QStringList, values);
-    QFETCH(QStringList, files);
     QFETCH(QString, input);
     QFETCH(QString, output);
 
-    QDir baseDir(BASE_DIR);
     QStringList lines = input.split(QLatin1String("\n"));
     QStringList vars; vars << QLatin1String("SOURCES");
 
     ProFileParser parser(0, &parseHandler);
     ProFile *proFile = parser.parsedProFile(QLatin1String(BASE_DIR "/test.pro"), false, &input);
     QVERIFY(proFile);
-    if (add)
+    Qt4ProjectManager::Internal::ProWriter::removeVarValues(proFile, &lines, values, vars);
-        Qt4ProjectManager::Internal::ProWriter::addFiles(proFile, &lines, baseDir, files, vars.at(0));
-    else
-        Qt4ProjectManager::Internal::ProWriter::removeFiles(proFile, &lines, baseDir, files, vars);
 
     QCOMPARE(lines.join(QLatin1String("\n")), output);
 }
@@ -316,5 +503,47 @@ void tst_ProFileWriter::multiVar()
     QCOMPARE(lines.join(QLatin1String("\n")), output);
 }
 
+void tst_ProFileWriter::addFiles()
+{
+    QString input = QLatin1String(
+            "SOURCES = foo.cpp"
+            );
+    QStringList lines = input.split(QLatin1String("\n"));
+    QString output = QLatin1String(
+            "SOURCES = foo.cpp \\\n"
+            "    sub/bar.cpp"
+            );
+
+    ProFileParser parser(0, &parseHandler);
+    ProFile *proFile = parser.parsedProFile(QLatin1String(BASE_DIR "/test.pro"), false, &input);
+    QVERIFY(proFile);
+    Qt4ProjectManager::Internal::ProWriter::addFiles(proFile, &lines, QDir(BASE_DIR),
+            QStringList() << QString::fromLatin1(BASE_DIR "/sub/bar.cpp"),
+            QLatin1String("SOURCES"));
+
+    QCOMPARE(lines.join(QLatin1String("\n")), output);
+}
+
+void tst_ProFileWriter::removeFiles()
+{
+    QString input = QLatin1String(
+            "SOURCES = foo.cpp sub/bar.cpp"
+            );
+    QStringList lines = input.split(QLatin1String("\n"));
+    QString output = QLatin1String(
+            "SOURCES = foo.cpp"
+            );
+
+    ProFileParser parser(0, &parseHandler);
+    ProFile *proFile = parser.parsedProFile(QLatin1String(BASE_DIR "/test.pro"), false, &input);
+    QVERIFY(proFile);
+    Qt4ProjectManager::Internal::ProWriter::removeFiles(proFile, &lines, QDir(BASE_DIR),
+            QStringList() << QString::fromLatin1(BASE_DIR "/sub/bar.cpp"),
+            QStringList() << QLatin1String("SOURCES") << QLatin1String("HEADERS"));
+
+    QCOMPARE(lines.join(QLatin1String("\n")), output);
+}
+
+
 QTEST_MAIN(tst_ProFileWriter)
 #include "tst_profilewriter.moc"