C++: Preserve original indentation in extract function

Also extend the refactoring changes to allow for reindenting.

Task-number: QTCREATORBUG-6797
Change-Id: I515c9a37d9e62e1d5de52ff99bd492e739a81885
Reviewed-by: Roberto Raggi <roberto.raggi@nokia.com>

src/plugins/cppeditor/cppinsertdecldef.cpp

@@ -433,27 +433,51 @@ public:
             funcDef.append(QLatin1String(" const"));
             funcDecl.append(QLatin1String(" const"));
         }
-        funcDef.append(QLatin1String("\n{\n")
+        funcDef.append(QLatin1String("\n{\n"));
-                       % currentFile->textOf(m_extractionStart, m_extractionEnd)
-                       % QLatin1Char('\n'));
         if (matchingClass)
             funcDecl.append(QLatin1String(";\n"));
         if (m_funcReturn) {
-            funcDef.append(QLatin1String("return ")
+            funcDef.append(QLatin1String("\nreturn ")
                         % m_relevantDecls.at(0).first
-                        % QLatin1String(";\n"));
+                        % QLatin1String(";"));
             funcCall.prepend(m_relevantDecls.at(0).second % QLatin1String(" = "));
         }
-        funcDef.append(QLatin1String("}\n\n"));
+        funcDef.append(QLatin1String("\n}\n\n"));
         funcDef.replace(QChar::ParagraphSeparator, QLatin1String("\n"));
         funcCall.append(QLatin1Char(';'));
 
+        // Get starting indentation from original code.
+        int indentedExtractionStart = m_extractionStart;
+        QChar current = currentFile->document()->characterAt(indentedExtractionStart - 1);
+        while (current == QLatin1Char(' ') || current == QLatin1Char('\t')) {
+            --indentedExtractionStart;
+            current = currentFile->document()->characterAt(indentedExtractionStart - 1);
+        }
+        QString extract = currentFile->textOf(indentedExtractionStart, m_extractionEnd);
+        extract.replace(QChar::ParagraphSeparator, QLatin1String("\n"));
+        if (!extract.endsWith(QLatin1Char('\n')) && m_funcReturn)
+            extract.append(QLatin1Char('\n'));
+
+        // FIXME: There are a couple related issues on the refactoring interface. You cannot
+        // simply have distinct indentation ranges within a chunk of content to be added, regardless
+        // of whether this is done through more than on change. That's why we actually apply twice,
+        // initially indenting the definition stub and the reindenting the original extraction.
+        // More details on the refactorings impl. where ranges are transformed into cursors.
         Utils::ChangeSet change;
         int position = currentFile->startOf(m_refFuncDef);
         change.insert(position, funcDef);
         change.replace(m_extractionStart, m_extractionEnd, funcCall);
         currentFile->setChangeSet(change);
-        currentFile->appendIndentRange(Utils::ChangeSet::Range(position, position + funcDef.length()));
+        currentFile->appendIndentRange(Utils::ChangeSet::Range(position, position + 1));
+        currentFile->apply();
+
+        QTextCursor tc = currentFile->document()->find(QLatin1String("{"), position);
+        QTC_ASSERT(!tc.isNull(), return);
+        position = tc.position() + 2;
+        change.clear();
+        change.insert(position, extract);
+        currentFile->setChangeSet(change);
+        currentFile->appendReindentRange(Utils::ChangeSet::Range(position, position + 1));
         currentFile->apply();
 
         // Write declaration, if necessary.
@@ -468,8 +492,7 @@ public:
             position = declFile->position(location.line(), location.column());
             change.insert(position, funcDecl);
             declFile->setChangeSet(change);
-            declFile->appendIndentRange(Utils::ChangeSet::Range(position,
+            declFile->appendIndentRange(Utils::ChangeSet::Range(position, position + 1));
-                                                                position + funcDecl.length()));
             declFile->apply();
         }
     }

src/plugins/cpptools/cpprefactoringchanges.cpp

@@ -32,6 +32,7 @@
 
 #include "cpprefactoringchanges.h"
 #include "cppcodestylepreferences.h"
+#include "cppqtstyleindenter.h"
 
 #include <TranslationUnit.h>
 #include <AST.h>
@@ -62,28 +63,22 @@ public:
                                  const QString &fileName,
                                  const TextEditor::BaseTextEditorWidget *textEditor) const
     {
-        // ### shares code with CPPEditor::indent()
-        QTextDocument *doc = selection.document();
-
-        QTextBlock block = doc->findBlock(selection.selectionStart());
-        const QTextBlock end = doc->findBlock(selection.selectionEnd()).next();
-
         const TextEditor::TabSettings &tabSettings =
             ProjectExplorer::actualTabSettings(fileName, textEditor);
-        // TODO: add similar method like above one
-        CppTools::QtStyleCodeFormatter codeFormatter(tabSettings,
-            CppToolsSettings::instance()->cppCodeStyle()->codeStyleSettings());
-        codeFormatter.updateStateUntil(block);
 
-        do {
+        CppQtStyleIndenter indenter;
-            int indent;
+        indenter.indent(selection.document(), selection, QChar::Null, tabSettings);
-            int padding;
+    }
-            codeFormatter.indentFor(block, &indent, &padding);
-            tabSettings.indentLine(block, indent + padding, padding);
-            codeFormatter.updateLineStateChange(block);
-            block = block.next();
-        } while (block.isValid() && block != end);
 
+    virtual void reindentSelection(const QTextCursor &selection,
+                                   const QString &fileName,
+                                   const TextEditor::BaseTextEditorWidget *textEditor) const
+    {
+        const TextEditor::TabSettings &tabSettings =
+            ProjectExplorer::actualTabSettings(fileName, textEditor);
+
+        CppQtStyleIndenter indenter;
+        indenter.reindent(selection.document(), selection, tabSettings);
     }
 
     virtual void fileChanged(const QString &fileName)

src/plugins/qmljstools/qmljsrefactoringchanges.cpp

@@ -34,6 +34,7 @@
 #include "qmljsqtstylecodeformatter.h"
 #include "qmljstoolsconstants.h"
 #include "qmljsmodelmanager.h"
+#include "qmljsindenter.h"
 
 #include <qmljs/parser/qmljsast_p.h>
 #include <qmljs/qmljsmodelmanagerinterface.h>
@@ -77,6 +78,17 @@ public:
         } while (block.isValid() && block != end);
     }
 
+    virtual void reindentSelection(const QTextCursor &selection,
+                                   const QString &fileName,
+                                   const TextEditor::BaseTextEditorWidget *textEditor) const
+    {
+        const TextEditor::TabSettings &tabSettings =
+            ProjectExplorer::actualTabSettings(fileName, textEditor);
+
+        QmlJSEditor::Internal::Indenter indenter;
+        indenter.reindent(selection.document(), selection, tabSettings);
+    }
+
     virtual void fileChanged(const QString &fileName)
     {
         m_modelManager->updateSourceFiles(QStringList(fileName), true);

src/plugins/texteditor/refactoringchanges.cpp

@@ -79,6 +79,15 @@ QList<QTextCursor> RefactoringChanges::rangesToSelections(QTextDocument *documen
 
     foreach (const Range &range, ranges) {
         QTextCursor selection(document);
+        // FIXME: The subtraction below for the start range might create a selection on a different
+        // block, which could cause unexpected effects on indentation for example when the range is
+        // precisely calculate. Since this cursor moves when content is inserted, it might not be
+        // possible to compensate for such a difference in advance because the value could be
+        // negative (which would eventually be right after content is inserted) and then taken as 0.
+        // A proper way for allowing fine granularly specified ranges would be to have two cursors
+        // and the first one with *keepPositionOnInsert*, for example, like it's done for the text
+        // editor overlay.
+
         // ### workaround for moving the textcursor when inserting text at the beginning of the range.
         selection.setPosition(qMax(0, range.start - 1));
         selection.setPosition(qMin(range.end, document->characterCount() - 1), QTextCursor::KeepAnchor);
@@ -175,6 +184,7 @@ RefactoringFile::RefactoringFile(QTextDocument *document, const QString &fileNam
     , m_openEditor(false)
     , m_activateEditor(false)
     , m_editorCursorPosition(-1)
+    , m_appliedOnce(false)
 { }
 
 RefactoringFile::RefactoringFile(BaseTextEditorWidget *editor)
@@ -184,6 +194,7 @@ RefactoringFile::RefactoringFile(BaseTextEditorWidget *editor)
     , m_openEditor(false)
     , m_activateEditor(false)
     , m_editorCursorPosition(-1)
+    , m_appliedOnce(false)
 { }
 
 RefactoringFile::RefactoringFile(const QString &fileName, const QSharedPointer<RefactoringChangesData> &data)
@@ -194,6 +205,7 @@ RefactoringFile::RefactoringFile(const QString &fileName, const QSharedPointer<R
     , m_openEditor(false)
     , m_activateEditor(false)
     , m_editorCursorPosition(-1)
+    , m_appliedOnce(false)
 {
     m_editor = RefactoringChanges::editorForFile(fileName);
 }
@@ -312,6 +324,14 @@ void RefactoringFile::appendIndentRange(const Range &range)
     m_indentRanges.append(range);
 }
 
+void RefactoringFile::appendReindentRange(const Range &range)
+{
+    if (m_fileName.isEmpty())
+        return;
+
+    m_reindentRanges.append(range);
+}
+
 void RefactoringFile::setOpenEditor(bool activate, int pos)
 {
     m_openEditor = true;
@@ -335,39 +355,45 @@ void RefactoringFile::apply()
     // apply changes, if any
     if (m_data && !(m_indentRanges.isEmpty() && m_changes.isEmpty())) {
         QTextDocument *doc = mutableDocument();
-        if (!doc)
+        if (doc) {
-            return;
-
-        {
             QTextCursor c = cursor();
-            c.beginEditBlock();
+            if (m_appliedOnce)
+                c.joinPreviousEditBlock();
+            else
+                c.beginEditBlock();
 
             // build indent selections now, applying the changeset will change locations
             const QList<QTextCursor> &indentSelections =
-                    RefactoringChanges::rangesToSelections(
+                    RefactoringChanges::rangesToSelections(doc, m_indentRanges);
-                            doc, m_indentRanges);
             m_indentRanges.clear();
+            const QList<QTextCursor> &reindentSelections =
+                    RefactoringChanges::rangesToSelections(doc, m_reindentRanges);
+            m_reindentRanges.clear();
 
             // apply changes and reindent
             m_changes.apply(&c);
             m_changes.clear();
-            foreach (const QTextCursor &selection, indentSelections) {
+
+            foreach (const QTextCursor &selection, indentSelections)
                 m_data->indentSelection(selection, m_fileName, m_editor);
-            }
+            foreach (const QTextCursor &selection, reindentSelections)
+                m_data->reindentSelection(selection, m_fileName, m_editor);
 
             c.endEditBlock();
-        }
 
-        // if this document doesn't have an editor, write the result to a file
+            // if this document doesn't have an editor, write the result to a file
-        if (!m_editor && m_textFileFormat.codec) {
+            if (!m_editor && m_textFileFormat.codec) {
-            QTC_ASSERT(!m_fileName.isEmpty(), return);
+                QTC_ASSERT(!m_fileName.isEmpty(), return);
-            QString error;
+                QString error;
-            if (!m_textFileFormat.writeFile(m_fileName, doc->toPlainText(), &error))
+                if (!m_textFileFormat.writeFile(m_fileName, doc->toPlainText(), &error))
-                qWarning() << "Could not apply changes to" << m_fileName << ". Error: " << error;
+                    qWarning() << "Could not apply changes to" << m_fileName << ". Error: " << error;
-        }
+            }
 
-        fileChanged();
+            fileChanged();
+        }
     }
+
+    m_appliedOnce = true;
 }
 
 void RefactoringFile::fileChanged()
@@ -384,6 +410,11 @@ void RefactoringChangesData::indentSelection(const QTextCursor &, const QString
     qWarning() << Q_FUNC_INFO << "not implemented";
 }
 
+void RefactoringChangesData::reindentSelection(const QTextCursor &, const QString &, const BaseTextEditorWidget *) const
+{
+    qWarning() << Q_FUNC_INFO << "not implemented";
+}
+
 void RefactoringChangesData::fileChanged(const QString &)
 {
 }

src/plugins/texteditor/refactoringchanges.h

@@ -80,6 +80,7 @@ public:
 
     void setChangeSet(const Utils::ChangeSet &changeSet);
     void appendIndentRange(const Range &range);
+    void appendReindentRange(const Range &range);
     void setOpenEditor(bool activate = false, int pos = -1);
     void apply();
 
@@ -103,9 +104,11 @@ protected:
     BaseTextEditorWidget *m_editor;
     Utils::ChangeSet m_changes;
     QList<Range> m_indentRanges;
+    QList<Range> m_reindentRanges;
     bool m_openEditor;
     bool m_activateEditor;
     int m_editorCursorPosition;
+    bool m_appliedOnce;
 
     friend class RefactoringChanges; // access to constructor
 };
@@ -154,6 +157,9 @@ public:
     virtual void indentSelection(const QTextCursor &selection,
                                  const QString &fileName,
                                  const BaseTextEditorWidget *textEditor) const;
+    virtual void reindentSelection(const QTextCursor &selection,
+                                   const QString &fileName,
+                                   const BaseTextEditorWidget *textEditor) const;
     virtual void fileChanged(const QString &fileName);
 };