Beautifier: Refactor formatting API

Break former monolithic methods into modular ones and re-introduce
synchronous formatting.

Change-Id: Ic4d8cbe451f028c7a3677570242cff9a2e362384
Reviewed-by: Eike Ziller <eike.ziller@theqtcompany.com>
Reviewed-by: Marcel Mathis <marcel.mathis@komaxgroup.com>

2 files changed, 183 insertions, 169 deletions

diff --git a/src/plugins/beautifier/beautifierplugin.cpp b/src/plugins/beautifier/beautifierplugin.cpp
index 8f39ef668e..cbd0b96b8b 100644
--- a/src/plugins/beautifier/beautifierplugin.cpp
+++ b/src/plugins/beautifier/beautifierplugin.cpp
@@ -67,206 +67,211 @@ using namespace TextEditor;
 namespace Beautifier {
 namespace Internal {
 
-BeautifierPlugin::BeautifierPlugin() :
-    m_asyncFormatMapper(new QSignalMapper)
+FormatTask format(FormatTask task)
 {
-    connect(m_asyncFormatMapper,
-            static_cast<void (QSignalMapper::*)(QObject *)>(&QSignalMapper::mapped),
-            this, &BeautifierPlugin::formatCurrentFileContinue);
-    connect(this, &BeautifierPlugin::pipeError, this, &BeautifierPlugin::showError);
-}
-
-BeautifierPlugin::~BeautifierPlugin()
-{
-    m_asyncFormatMapper->deleteLater();
-}
-
-bool BeautifierPlugin::initialize(const QStringList &arguments, QString *errorString)
-{
-    Q_UNUSED(arguments)
-    Q_UNUSED(errorString)
-
-    m_tools << new ArtisticStyle::ArtisticStyle(this);
-    m_tools << new ClangFormat::ClangFormat(this);
-    m_tools << new Uncrustify::Uncrustify(this);
-
-    Core::ActionContainer *menu = Core::ActionManager::createMenu(Constants::MENU_ID);
-    menu->menu()->setTitle(QCoreApplication::translate("Beautifier", Constants::OPTION_TR_CATEGORY));
-    Core::ActionManager::actionContainer(Core::Constants::M_TOOLS)->addMenu(menu);
-
-    foreach (BeautifierAbstractTool *tool, m_tools) {
-        tool->initialize();
-        const QList<QObject *> autoReleasedObjects = tool->autoReleaseObjects();
-        foreach (QObject *object, autoReleasedObjects)
-            addAutoReleasedObject(object);
-    }
-
-    // The single shot is needed, otherwise the menu will stay disabled even
-    // when the submenu's actions get enabled later on.
-    QTimer::singleShot(0, this, SLOT(updateActions()));
-    return true;
-}
+    task.error.clear();
+    task.formattedData.clear();
 
-void BeautifierPlugin::extensionsInitialized()
-{
-    if (const Core::EditorManager *editorManager = Core::EditorManager::instance()) {
-        connect(editorManager, &Core::EditorManager::currentEditorChanged,
-                this, &BeautifierPlugin::updateActions);
-    }
-}
-
-ExtensionSystem::IPlugin::ShutdownFlag BeautifierPlugin::aboutToShutdown()
-{
-    return SynchronousShutdown;
-}
-
-void BeautifierPlugin::updateActions(Core::IEditor *editor)
-{
-    foreach (BeautifierAbstractTool *tool, m_tools)
-        tool->updateActions(editor);
-}
-
-// Use pipeError() instead of calling showError() because this function may run in another thread.
-QString BeautifierPlugin::format(const QString &text, const Command &command,
-                                 const QString &fileName, bool *timeout)
-{
-    const QString executable = command.executable();
+    const QString executable = task.command.executable();
     if (executable.isEmpty())
-        return QString();
+        return task;
 
-    switch (command.processing()) {
+    switch (task.command.processing()) {
     case Command::FileProcessing: {
         // Save text to temporary file
-        const QFileInfo fi(fileName);
+        const QFileInfo fi(task.filePath);
         Utils::TempFileSaver sourceFile(QDir::tempPath() + QLatin1String("/qtc_beautifier_XXXXXXXX.")
                                         + fi.suffix());
         sourceFile.setAutoRemove(true);
-        sourceFile.write(text.toUtf8());
+        sourceFile.write(task.sourceData.toUtf8());
         if (!sourceFile.finalize()) {
-            emit pipeError(tr("Cannot create temporary file \"%1\": %2.")
-                           .arg(sourceFile.fileName()).arg(sourceFile.errorString()));
-            return QString();
+            task.error = QObject::tr("Cannot create temporary file \"%1\": %2.")
+                    .arg(sourceFile.fileName()).arg(sourceFile.errorString());
+            return task;
         }
 
         // Format temporary file
         QProcess process;
-        QStringList options = command.options();
+        QStringList options = task.command.options();
         options.replaceInStrings(QLatin1String("%file"), sourceFile.fileName());
         process.start(executable, options);
         if (!process.waitForFinished(5000)) {
-            if (timeout)
-                *timeout = true;
             process.kill();
-            emit pipeError(tr("Cannot call %1 or some other error occurred.").arg(executable));
-            return QString();
+            task.error = QObject::tr("Cannot call %1 or some other error occurred. Time out "
+                                     "reached while formatting file %2.")
+                    .arg(executable).arg(task.filePath);
+            return task;
         }
         const QByteArray output = process.readAllStandardError();
         if (!output.isEmpty())
-            emit pipeError(executable + QLatin1String(": ") + QString::fromUtf8(output));
+            task.error = executable + QLatin1String(": ") + QString::fromUtf8(output);
 
         // Read text back
         Utils::FileReader reader;
         if (!reader.fetch(sourceFile.fileName(), QIODevice::Text)) {
-            emit pipeError(tr("Cannot read file \"%1\": %2.")
-                           .arg(sourceFile.fileName()).arg(reader.errorString()));
-            return QString();
+            task.error = QObject::tr("Cannot read file \"%1\": %2.")
+                    .arg(sourceFile.fileName()).arg(reader.errorString());
+            return task;
         }
-        return QString::fromUtf8(reader.data());
+        task.formattedData = QString::fromUtf8(reader.data());
+        return task;
     } break;
 
     case Command::PipeProcessing: {
         QProcess process;
-        QStringList options = command.options();
-        options.replaceInStrings(QLatin1String("%file"), fileName);
+        QStringList options = task.command.options();
+        options.replaceInStrings(QLatin1String("%file"), task.filePath);
         process.start(executable, options);
         if (!process.waitForStarted(3000)) {
-            emit pipeError(tr("Cannot call %1 or some other error occurred.").arg(executable));
-            return QString();
+            task.error = QObject::tr("Cannot call %1 or some other error occurred.")
+                    .arg(executable);
+            return task;
         }
-        process.write(text.toUtf8());
+        process.write(task.sourceData.toUtf8());
         process.closeWriteChannel();
         if (!process.waitForFinished(5000)) {
-            if (timeout)
-                *timeout = true;
             process.kill();
-            emit pipeError(tr("Cannot call %1 or some other error occurred.").arg(executable));
-            return QString();
+            task.error = QObject::tr("Cannot call %1 or some other error occurred. Time out "
+                                     "reached while formatting file %2.")
+                    .arg(executable).arg(task.filePath);
+            return task;
         }
         const QByteArray errorText = process.readAllStandardError();
         if (!errorText.isEmpty()) {
-            emit pipeError(QString::fromLatin1("%1: %2").arg(executable)
-                           .arg(QString::fromUtf8(errorText)));
-            return QString();
+            task.error = QString::fromLatin1("%1: %2").arg(executable)
+                    .arg(QString::fromUtf8(errorText));
+            return task;
         }
 
-        const bool addsNewline = command.pipeAddsNewline();
-        const bool returnsCRLF = command.returnsCRLF();
+        const bool addsNewline = task.command.pipeAddsNewline();
+        const bool returnsCRLF = task.command.returnsCRLF();
         if (addsNewline || returnsCRLF) {
-            QString formatted = QString::fromUtf8(process.readAllStandardOutput());
+            task.formattedData = QString::fromUtf8(process.readAllStandardOutput());
             if (addsNewline)
-                formatted.remove(QRegExp(QLatin1String("(\\r\\n|\\n)$")));
+                task.formattedData.remove(QRegExp(QLatin1String("(\\r\\n|\\n)$")));
             if (returnsCRLF)
-                formatted.replace(QLatin1String("\r\n"), QLatin1String("\n"));
-            return formatted;
+                task.formattedData.replace(QLatin1String("\r\n"), QLatin1String("\n"));
+            return task;
         }
-        return QString::fromUtf8(process.readAllStandardOutput());
+        task.formattedData = QString::fromUtf8(process.readAllStandardOutput());
+        return task;
     }
     }
 
-    return QString();
+    return task;
 }
 
-void BeautifierPlugin::formatCurrentFile(const Command &command, int startPos, int endPos)
+QString sourceData(TextEditorWidget *editor, int startPos, int endPos)
 {
-    QTC_ASSERT(startPos <= endPos, return);
+    return (startPos < 0)
+            ? editor->toPlainText()
+            : Convenience::textAt(editor->textCursor(), startPos, (endPos - startPos));
+}
 
-    if (TextEditorWidget *widget = TextEditorWidget::currentTextEditorWidget()) {
-        if (const TextDocument *doc = widget->textDocument()) {
-            const QString sourceData = (startPos < 0)
-                    ? widget->toPlainText()
-                    : Convenience::textAt(widget->textCursor(), startPos, (endPos - startPos));
-            if (sourceData.isEmpty())
-                return;
-            const FormatTask task = FormatTask(widget, doc->filePath().toString(), sourceData,
-                                               command, startPos, endPos);
-
-            QFutureWatcher<FormatTask> *watcher = new QFutureWatcher<FormatTask>;
-            connect(doc, &TextDocument::contentsChanged,
-                    watcher, &QFutureWatcher<FormatTask>::cancel);
-            connect(watcher, &QFutureWatcherBase::finished, m_asyncFormatMapper,
-                    static_cast<void (QSignalMapper::*)()>(&QSignalMapper::map));
-            m_asyncFormatMapper->setMapping(watcher, watcher);
-            watcher->setFuture(Utils::runAsync(&BeautifierPlugin::formatAsync, this, task));
-        }
+bool BeautifierPlugin::initialize(const QStringList &arguments, QString *errorString)
+{
+    Q_UNUSED(arguments)
+    Q_UNUSED(errorString)
+
+    m_tools << new ArtisticStyle::ArtisticStyle(this);
+    m_tools << new ClangFormat::ClangFormat(this);
+    m_tools << new Uncrustify::Uncrustify(this);
+
+    Core::ActionContainer *menu = Core::ActionManager::createMenu(Constants::MENU_ID);
+    menu->menu()->setTitle(QCoreApplication::translate("Beautifier", Constants::OPTION_TR_CATEGORY));
+    Core::ActionManager::actionContainer(Core::Constants::M_TOOLS)->addMenu(menu);
+
+    foreach (BeautifierAbstractTool *tool, m_tools) {
+        tool->initialize();
+        const QList<QObject *> autoReleasedObjects = tool->autoReleaseObjects();
+        foreach (QObject *object, autoReleasedObjects)
+            addAutoReleasedObject(object);
     }
+
+    // The single shot is needed, otherwise the menu will stay disabled even
+    // when the submenu's actions get enabled later on.
+    QTimer::singleShot(0, this, SLOT(updateActions()));
+    return true;
+}
+
+void BeautifierPlugin::extensionsInitialized()
+{
+    if (const Core::EditorManager *editorManager = Core::EditorManager::instance()) {
+        connect(editorManager, &Core::EditorManager::currentEditorChanged,
+                this, &BeautifierPlugin::updateActions);
+    }
+}
+
+ExtensionSystem::IPlugin::ShutdownFlag BeautifierPlugin::aboutToShutdown()
+{
+    return SynchronousShutdown;
 }
 
-void BeautifierPlugin::formatAsync(QFutureInterface<FormatTask> &future, FormatTask task)
+void BeautifierPlugin::updateActions(Core::IEditor *editor)
 {
-    task.formattedData = format(task.sourceData, task.command, task.filePath, &task.timeout);
-    future.reportResult(task);
+    foreach (BeautifierAbstractTool *tool, m_tools)
+        tool->updateActions(editor);
 }
 
-void BeautifierPlugin::formatCurrentFileContinue(QObject *watcher)
+void BeautifierPlugin::formatCurrentFile(const Command &command, int startPos, int endPos)
 {
-    QFutureWatcher<FormatTask> *futureWatcher = static_cast<QFutureWatcher<FormatTask>*>(watcher);
-    if (!futureWatcher) {
-        if (watcher)
-            watcher->deleteLater();
+    if (TextEditorWidget *editor = TextEditorWidget::currentTextEditorWidget())
+        formatEditorAsync(editor, command, startPos, endPos);
+}
+
+/**
+ * Formats the text of @a editor using @a command. @a startPos and @a endPos specifies the range of
+ * the editor's text that will be formatted. If @a startPos is negative the editor's entire text is
+ * formatted.
+ *
+ * @pre @a endPos must be greater than or equal to @a startPos
+ */
+void BeautifierPlugin::formatEditor(TextEditorWidget *editor, const Command &command, int startPos,
+                                    int endPos)
+{
+    QTC_ASSERT(startPos <= endPos, return);
+
+    const QString sd = sourceData(editor, startPos, endPos);
+    if (sd.isEmpty())
         return;
-    }
+    checkAndApplyTask(format(FormatTask(editor, editor->textDocument()->filePath().toString(), sd,
+                                        command, startPos, endPos)));
+}
 
-    if (futureWatcher->isCanceled()) {
-        showError(tr("File was modified."));
-        futureWatcher->deleteLater();
+/**
+ * Behaves like formatEditor except that the formatting is done asynchronously.
+ */
+void BeautifierPlugin::formatEditorAsync(TextEditorWidget *editor, const Command &command,
+                                         int startPos, int endPos)
+{
+    QTC_ASSERT(startPos <= endPos, return);
+
+    const QString sd = sourceData(editor, startPos, endPos);
+    if (sd.isEmpty())
         return;
-    }
 
-    const FormatTask task = futureWatcher->result();
-    futureWatcher->deleteLater();
+    QFutureWatcher<FormatTask> *watcher = new QFutureWatcher<FormatTask>;
+    const TextDocument *doc = editor->textDocument();
+    connect(doc, &TextDocument::contentsChanged, watcher, &QFutureWatcher<FormatTask>::cancel);
+    connect(watcher, &QFutureWatcherBase::finished, [this, watcher]() {
+        if (watcher->isCanceled())
+            showError(tr("File was modified."));
+        else
+            checkAndApplyTask(watcher->result());
+        watcher->deleteLater();
+    });
+    watcher->setFuture(Utils::runAsync(&format, FormatTask(editor, doc->filePath().toString(), sd,
+                                                           command, startPos, endPos)));
+}
 
-    if (task.timeout) {
-        showError(tr("Time out reached while formatting file %1.").arg(task.filePath));
+/**
+ * Checks the state of @a task and if the formatting was successful calls updateEditorText() with
+ * the respective members of @a task.
+ */
+void BeautifierPlugin::checkAndApplyTask(const FormatTask &task)
+{
+    if (!task.error.isEmpty()) {
+        showError(task.error);
         return;
     }
 
@@ -281,19 +286,33 @@ void BeautifierPlugin::formatCurrentFileContinue(QObject *watcher)
         return;
     }
 
-    const QString sourceData = textEditor->toPlainText();
     const QString formattedData = (task.startPos < 0)
             ? task.formattedData
-            : QString(sourceData).replace(task.startPos, (task.endPos - task.startPos),
-                                          task.formattedData);
-    if (sourceData == formattedData)
+            : QString(textEditor->toPlainText()).replace(
+                  task.startPos, (task.endPos - task.startPos), task.formattedData);
+
+    updateEditorText(textEditor, formattedData);
+}
+
+/**
+ * Sets the text of @a editor to @a text. Instead of replacing the entire text, however, only the
+ * actually changed parts are updated while preserving the cursor position, the folded
+ * blocks, and the scroll bar position.
+ */
+void BeautifierPlugin::updateEditorText(QPlainTextEdit *editor, const QString &text)
+{
+    const QString editorText = editor->toPlainText();
+    if (editorText == text)
         return;
 
+    // Calculate diff
+    DiffEditor::Differ differ;
+    const QList<DiffEditor::Diff> diff = differ.diff(editorText, text);
 
     // Since QTextCursor does not work properly with folded blocks, all blocks must be unfolded.
     // To restore the current state at the end, keep track of which block is folded.
     QList<int> foldedBlocks;
-    QTextBlock block = textEditor->document()->firstBlock();
+    QTextBlock block = editor->document()->firstBlock();
     while (block.isValid()) {
         if (const TextBlockUserData *userdata = static_cast<TextBlockUserData *>(block.userData())) {
             if (userdata->folded()) {
@@ -303,18 +322,14 @@ void BeautifierPlugin::formatCurrentFileContinue(QObject *watcher)
         }
         block = block.next();
     }
-    textEditor->update();
+    editor->update();
 
     // Save the current viewport position of the cursor to ensure the same vertical position after
     // the formatted text has set to the editor.
-    int absoluteVerticalCursorOffset = textEditor->cursorRect().y();
-
-    // Calculate diff
-    DiffEditor::Differ differ;
-    const QList<DiffEditor::Diff> diff = differ.diff(sourceData, formattedData);
+    int absoluteVerticalCursorOffset = editor->cursorRect().y();
 
     // Update changed lines and keep track of the cursor position
-    QTextCursor cursor = textEditor->textCursor();
+    QTextCursor cursor = editor->textCursor();
     int charactersInfrontOfCursor = cursor.position();
     int newCursorPos = charactersInfrontOfCursor;
     cursor.beginEditBlock();
@@ -381,22 +396,22 @@ void BeautifierPlugin::formatCurrentFileContinue(QObject *watcher)
     }
     cursor.endEditBlock();
     cursor.setPosition(newCursorPos);
-    textEditor->setTextCursor(cursor);
+    editor->setTextCursor(cursor);
 
     // Adjust vertical scrollbar
-    absoluteVerticalCursorOffset = textEditor->cursorRect().y() - absoluteVerticalCursorOffset;
-    const double fontHeight = QFontMetrics(textEditor->document()->defaultFont()).height();
-    textEditor->verticalScrollBar()->setValue(textEditor->verticalScrollBar()->value()
+    absoluteVerticalCursorOffset = editor->cursorRect().y() - absoluteVerticalCursorOffset;
+    const double fontHeight = QFontMetrics(editor->document()->defaultFont()).height();
+    editor->verticalScrollBar()->setValue(editor->verticalScrollBar()->value()
                                               + absoluteVerticalCursorOffset / fontHeight);
     // Restore folded blocks
-    const QTextDocument *doc = textEditor->document();
+    const QTextDocument *doc = editor->document();
     foreach (const int blockId, foldedBlocks) {
         const QTextBlock block = doc->findBlockByNumber(qMax(0, blockId));
         if (block.isValid())
             TextDocumentLayout::doFoldOrUnfold(block, false);
     }
 
-    textEditor->document()->setModified(true);
+    editor->document()->setModified(true);
 }
 
 void BeautifierPlugin::showError(const QString &error)
diff --git a/src/plugins/beautifier/beautifierplugin.h b/src/plugins/beautifier/beautifierplugin.h
index 4d43cf8cbf..49dfd06690 100644
--- a/src/plugins/beautifier/beautifierplugin.h
+++ b/src/plugins/beautifier/beautifierplugin.h
@@ -36,6 +36,7 @@
 #include <QSignalMapper>
 
 namespace Core { class IEditor; }
+namespace TextEditor { class TextEditorWidget; }
 
 namespace Beautifier {
 namespace Internal {
@@ -44,6 +45,10 @@ class BeautifierAbstractTool;
 
 struct FormatTask
 {
+    FormatTask() :
+        startPos(-1),
+        endPos(0) {}
+
     FormatTask(QPlainTextEdit *_editor, const QString &_filePath, const QString &_sourceData,
                const Command &_command, int _startPos = -1, int _endPos = 0) :
         editor(_editor),
@@ -51,8 +56,7 @@ struct FormatTask
         sourceData(_sourceData),
         command(_command),
         startPos(_startPos),
-        endPos(_endPos),
-        timeout(false) {}
+        endPos(_endPos) {}
 
     QPointer<QPlainTextEdit> editor;
     QString filePath;
@@ -60,8 +64,8 @@ struct FormatTask
     Command command;
     int startPos;
     int endPos;
-    bool timeout;
     QString formattedData;
+    QString error;
 };
 
 class BeautifierPlugin : public ExtensionSystem::IPlugin
@@ -70,35 +74,30 @@ class BeautifierPlugin : public ExtensionSystem::IPlugin
     Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QtCreatorPlugin" FILE "Beautifier.json")
 
 public:
-    BeautifierPlugin();
-    ~BeautifierPlugin();
     bool initialize(const QStringList &arguments, QString *errorString) override;
     void extensionsInitialized() override;
     ShutdownFlag aboutToShutdown() override;
 
-    QString format(const QString &text, const Command &command, const QString &fileName,
-                   bool *timeout = 0);
     void formatCurrentFile(const Command &command, int startPos = -1, int endPos = 0);
-    void formatAsync(QFutureInterface<FormatTask> &future, FormatTask task);
 
     static QString msgCannotGetConfigurationFile(const QString &command);
     static QString msgFormatCurrentFile();
     static QString msgFormatSelectedText();
     static QString msgCommandPromptDialogTitle(const QString &command);
-
-public slots:
     static void showError(const QString &error);
 
 private slots:
     void updateActions(Core::IEditor *editor = 0);
-    void formatCurrentFileContinue(QObject *watcher = 0);
-
-signals:
-    void pipeError(QString);
 
 private:
     QList<BeautifierAbstractTool *> m_tools;
-    QSignalMapper *m_asyncFormatMapper;
+
+    void formatEditor(TextEditor::TextEditorWidget *editor, const Command &command,
+                      int startPos = -1, int endPos = 0);
+    void formatEditorAsync(TextEditor::TextEditorWidget *editor, const Command &command,
+                           int startPos = -1, int endPos = 0);
+    void checkAndApplyTask(const FormatTask &task);
+    void updateEditorText(QPlainTextEdit *editor, const QString &text);
 };
 
 } // namespace Internal