From 2bfa16daa16bd578eb83561f76e14ad55e38678d Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 19 Jun 2024 16:47:48 +0200 Subject: [PATCH] CppEditor: Fix code folding in the presence of ifdef-ed out blocks The intention was to not track brace depth and folding indent inside ifdefed-out code. However, the information about which blocks are ifdefed out come from the semantic highlighter, and the logic was in the syntax highlighter, which runs before the semantic highlighter and thus does not know which blocks are currently inactive. Therefore, we move the logic to the place where we receive the information from the semantic highlighter. Fixes: QTCREATORBUG-21064 Change-Id: I144f4d636b9b9a993763b45a2b6c6c96195a2f77 Reviewed-by: David Schulz --- src/plugins/cppeditor/cppeditordocument.cpp | 48 ++++----- src/plugins/cppeditor/cpphighlighter.cpp | 105 ++++++++++++++++++-- 2 files changed, 120 insertions(+), 33 deletions(-) diff --git a/src/plugins/cppeditor/cppeditordocument.cpp b/src/plugins/cppeditor/cppeditordocument.cpp index 49d272eb725..a9d331d2c88 100644 --- a/src/plugins/cppeditor/cppeditordocument.cpp +++ b/src/plugins/cppeditor/cppeditordocument.cpp @@ -325,38 +325,40 @@ void CppEditorDocument::setIfdefedOutBlocks(const QList QTextBlock block = document()->firstBlock(); bool needUpdate = false; int rangeNumber = 0; - int braceDepthDelta = 0; + int previousBraceDepth = 0; while (block.isValid()) { - bool cleared = false; - bool set = false; + bool resetToPrevious = false; if (rangeNumber < blocks.size()) { const BlockRange &range = blocks.at(rangeNumber); if (block.position() >= range.first() - && ((block.position() + block.length() - 1) <= range.last() || !range.last())) - set = TextDocumentLayout::setIfdefedOut(block); - else - cleared = TextDocumentLayout::clearIfdefedOut(block); + && ((block.position() + block.length() - 1) <= range.last() || !range.last())) { + TextDocumentLayout::setIfdefedOut(block); + resetToPrevious = true; + } else { + TextDocumentLayout::clearIfdefedOut(block); + previousBraceDepth = TextDocumentLayout::braceDepth(block); + resetToPrevious = false; + } if (block.contains(range.last())) ++rangeNumber; } else { - cleared = TextDocumentLayout::clearIfdefedOut(block); + TextDocumentLayout::clearIfdefedOut(block); + resetToPrevious = false; } - if (cleared || set) { - needUpdate = true; - int delta = TextDocumentLayout::braceDepthDelta(block); - if (cleared) - braceDepthDelta += delta; - else if (set) - braceDepthDelta -= delta; - } - - if (braceDepthDelta) { - qCDebug(highlighterLog) - << "changing brace depth and folding indent by" << braceDepthDelta << "for line" - << (block.blockNumber() + 1) << "due to ifdefed out code"; - TextDocumentLayout::changeBraceDepth(block,braceDepthDelta); - TextDocumentLayout::changeFoldingIndent(block, braceDepthDelta); // ### C++ only, refactor! + // Do not change brace depth and folding indent in ifdefed-out code. + if (resetToPrevious) { + const int currentBraceDepth = TextDocumentLayout::braceDepth(block); + const int currentFoldingIndent = TextDocumentLayout::foldingIndent(block); + if (currentBraceDepth != previousBraceDepth + || currentFoldingIndent != previousBraceDepth) { + TextDocumentLayout::setBraceDepth(block, previousBraceDepth); + TextDocumentLayout::setFoldingIndent(block, previousBraceDepth); + needUpdate = true; + qCDebug(highlighterLog) + << "changing brace depth and folding indent to" << previousBraceDepth + << "for line" << (block.blockNumber() + 1) << "in ifdefed out code"; + } } block = block.next(); diff --git a/src/plugins/cppeditor/cpphighlighter.cpp b/src/plugins/cppeditor/cpphighlighter.cpp index e96576c4825..5051657afff 100644 --- a/src/plugins/cppeditor/cpphighlighter.cpp +++ b/src/plugins/cppeditor/cpphighlighter.cpp @@ -21,7 +21,10 @@ #include #ifdef WITH_TESTS +#include "cppeditorwidget.h" +#include "cpptoolstestcase.h" #include +#include #endif using namespace TextEditor; @@ -280,16 +283,6 @@ void CppHighlighter::highlightBlock(const QString &text) TextDocumentLayout::setParentheses(currentBlock(), parentheses); - // if the block is ifdefed out, we only store the parentheses, but - // do not adjust the brace depth. - if (TextBlockUserData *userData = TextDocumentLayout::textUserData(currentBlock()); - userData && userData->ifdefedOut()) { - braceDepth = initialBraceDepth; - foldingIndent = initialBraceDepth; - qCDebug(highlighterLog) << "block is ifdefed out, resetting brace depth and folding indent to" - << initialBraceDepth; - } - TextDocumentLayout::setFoldingIndent(currentBlock(), foldingIndent); setCurrentBlockState((braceDepth << 8) | tokenize.state()); qCDebug(highlighterLog) << "storing brace depth" << braceDepth << "and folding indent" << foldingIndent; @@ -543,6 +536,8 @@ void CppHighlighter::highlightDoxygenComment(const QString &text, int position, namespace Internal { #ifdef WITH_TESTS +using namespace CppEditor::Tests; +using namespace Tests; class CppHighlighterTest : public CppHighlighter { Q_OBJECT @@ -733,12 +728,102 @@ private slots: private: QTextDocument m_doc; }; + +class CodeFoldingTest : public QObject +{ + Q_OBJECT + +private slots: + void test() + { + const QByteArray content = R"(cpp // 0,0 +int main() { // 1,0 +#if 0 // 1,1 + if (true) { // 1,1 + //... // 1,1 + } // 1,1 + else { // 1,1 + //... // 1,1 + } // 1,1 +#else // 1,1 + if (true) { // 2,1 + //... // 2,2 + } // 1,1 +#endif // 1,1 +} // 0,0 + // 0,0 +cpp)"; + TemporaryDir temporaryDir; + QVERIFY(temporaryDir.isValid()); + CppTestDocument testDocument("file.cpp", content); + testDocument.setBaseDirectory(temporaryDir.path()); + QVERIFY(testDocument.writeToDisk()); + + QVERIFY(TestCase::openCppEditor(testDocument.filePath(), &testDocument.m_editor, + &testDocument.m_editorWidget)); + + QEventLoop loop; + QTimer t; + t.setSingleShot(true); + connect(&t, &QTimer::timeout, &loop, [&] {loop.exit(1); }); + const auto check = [&] { + const struct LoopHandler { + LoopHandler(QEventLoop &loop) : loop(loop) {} + ~LoopHandler() { loop.quit(); } + + private: + QEventLoop &loop; + } loopHandler(loop); + + const auto getExpectedBraceDepthAndFoldingIndent = [](const QTextBlock &block) { + const QString &text = block.text(); + if (text.size() < 3) + return std::make_pair(-1, -1); + bool ok; + const int braceDepth = text.mid(text.size() - 3, 1).toInt(&ok); + if (!ok) + return std::make_pair(-1, -1); + const int foldingIndent = text.last(1).toInt(&ok); + if (!ok) + return std::make_pair(-1, -1); + return std::make_pair(braceDepth, foldingIndent); + }; + const auto getActualBraceDepthAndFoldingIndent = [](const QTextBlock &block) { + const int braceDepth = block.userState() >> 8; + const int foldingIndent = TextDocumentLayout::foldingIndent(block); + return std::make_pair(braceDepth, foldingIndent); + }; + TextDocument * const doc = testDocument.m_editorWidget->textDocument(); + const QTextBlock lastBlock = doc->document()->lastBlock(); + for (QTextBlock b = doc->document()->firstBlock(); b.isValid() && b != lastBlock; + b = b.next()) { + const auto actual = getActualBraceDepthAndFoldingIndent(b); + const auto expected = getExpectedBraceDepthAndFoldingIndent(b); + if (actual != expected) + qDebug() << "In line" << (b.blockNumber() + 1); + QCOMPARE(actual, expected); + } + }; + connect(testDocument.m_editorWidget, &CppEditorWidget::ifdefedOutBlocksChanged, + this, check); + t.start(5000); + QCOMPARE(loop.exec(), 0); + } + + void cleanup() + { + QVERIFY(Core::EditorManager::closeAllEditors(false)); + QVERIFY(TestCase::garbageCollectGlobalSnapshot()); + } +}; + #endif // WITH_TESTS void registerHighlighterTests(ExtensionSystem::IPlugin &plugin) { #ifdef WITH_TESTS plugin.addTest(); + plugin.addTest(); #else Q_UNUSED(plugin) #endif