QTextMarkdownImporter: don't keep heading level on following list item

When reading a document like

 # heading
 - list item

and then re-writing it, it turned into

 # heading
 - # list item

because QTextCursor::insertList() simply calls QTextCursor::insertBlock(), thus
inheriting block format from the previous block, without an opportunity to
explicitly define the block format.  So be more consistent: use
QTextMarkdownImporter::insertBlock() for blocks inside list items too.  Now it
fully defines blockFormat first, then inserts the block, and then adds it to
the current list only when the "paragraph" is actually the list item's text
(but not when it's a continuation paragraph).  Also, be prepared for applying
and removing block markers to arbitrary blocks, just in case (they might be
useful for block quotes, for example).

Change-Id: I391820af9b65e75abce12abab45d2477c49c86ac
Reviewed-by: Gatis Paeglis <gatis.paeglis@qt.io>
 
 
 
 
 
 

src/gui/text/qtextmarkdownimporter.cpp

@@ -151,25 +151,16 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det)
     m_blockType = blockType;
     switch (blockType) {
     case MD_BLOCK_P:
-        if (m_listStack.isEmpty()) {
-            m_needsInsertBlock = true;
+        if (!m_listStack.isEmpty())
+            qCDebug(lcMD, m_listItem ? "P of LI at level %d"  : "P continuation inside LI at level %d", m_listStack.count());
+        else
             qCDebug(lcMD, "P");
-        } else {
-            if (m_emptyListItem) {
-                qCDebug(lcMD, "LI text block at level %d -> BlockIndent %d",
-                        m_listStack.count(), m_cursor->blockFormat().indent());
-                m_emptyListItem = false;
-            } else {
-                qCDebug(lcMD, "P inside LI at level %d", m_listStack.count());
-                m_needsInsertBlock = true;
-            }
-        }
+        m_needsInsertBlock = true;
         break;
-    case MD_BLOCK_QUOTE: {
+    case MD_BLOCK_QUOTE:
         ++m_blockQuoteDepth;
         qCDebug(lcMD, "QUOTE level %d", m_blockQuoteDepth);
         break;
-    }
     case MD_BLOCK_CODE: {
         MD_BLOCK_CODE_DETAIL *detail = static_cast<MD_BLOCK_CODE_DETAIL *>(det);
         m_codeBlock = true;
@@ -194,51 +185,40 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det)
         qCDebug(lcMD, "H%d", detail->level);
     } break;
     case MD_BLOCK_LI: {
-        m_needsInsertBlock = false;
-        MD_BLOCK_LI_DETAIL *detail = static_cast<MD_BLOCK_LI_DETAIL *>(det);
-        QTextList *list = m_listStack.top();
-        QTextBlockFormat bfmt = list->item(list->count() - 1).blockFormat();
-        bfmt.setMarker(detail->is_task ?
-                           (detail->task_mark == ' ' ? QTextBlockFormat::Unchecked : QTextBlockFormat::Checked) :
-                           QTextBlockFormat::NoMarker);
-        if (!m_emptyList) {
-            m_cursor->insertBlock(bfmt, QTextCharFormat());
-            list->add(m_cursor->block());
-        }
-        m_cursor->setBlockFormat(bfmt);
-        qCDebug(lcMD) << (m_emptyList ? "LI (first in list)" : "LI");
-        m_emptyList = false; // Avoid insertBlock for the first item (because insertList already did that)
+        m_needsInsertBlock = true;
         m_listItem = true;
-        m_emptyListItem = true;
+        MD_BLOCK_LI_DETAIL *detail = static_cast<MD_BLOCK_LI_DETAIL *>(det);
+        m_markerType = detail->is_task ?
+                    (detail->task_mark == ' ' ? QTextBlockFormat::Unchecked : QTextBlockFormat::Checked) :
+                    QTextBlockFormat::NoMarker;
+        qCDebug(lcMD) << "LI";
     } break;
     case MD_BLOCK_UL: {
         MD_BLOCK_UL_DETAIL *detail = static_cast<MD_BLOCK_UL_DETAIL *>(det);
-        QTextListFormat fmt;
-        fmt.setIndent(m_listStack.count() + 1);
+        m_listFormat = QTextListFormat();
+        m_listFormat.setIndent(m_listStack.count() + 1);
         switch (detail->mark) {
         case '*':
-            fmt.setStyle(QTextListFormat::ListCircle);
+            m_listFormat.setStyle(QTextListFormat::ListCircle);
             break;
         case '+':
-            fmt.setStyle(QTextListFormat::ListSquare);
+            m_listFormat.setStyle(QTextListFormat::ListSquare);
             break;
         default: // including '-'
-            fmt.setStyle(QTextListFormat::ListDisc);
+            m_listFormat.setStyle(QTextListFormat::ListDisc);
             break;
         }
         qCDebug(lcMD, "UL %c level %d", detail->mark, m_listStack.count());
-        m_listStack.push(m_cursor->insertList(fmt));
-        m_emptyList = true;
+        m_needsInsertList = true;
     } break;
     case MD_BLOCK_OL: {
         MD_BLOCK_OL_DETAIL *detail = static_cast<MD_BLOCK_OL_DETAIL *>(det);
-        QTextListFormat fmt;
-        fmt.setIndent(m_listStack.count() + 1);
-        fmt.setNumberSuffix(QChar::fromLatin1(detail->mark_delimiter));
-        fmt.setStyle(QTextListFormat::ListDecimal);
+        m_listFormat = QTextListFormat();
+        m_listFormat.setIndent(m_listStack.count() + 1);
+        m_listFormat.setNumberSuffix(QChar::fromLatin1(detail->mark_delimiter));
+        m_listFormat.setStyle(QTextListFormat::ListDecimal);
         qCDebug(lcMD, "OL xx%d level %d", detail->mark_delimiter, m_listStack.count());
-        m_listStack.push(m_cursor->insertList(fmt));
-        m_emptyList = true;
+        m_needsInsertList = true;
     } break;
     case MD_BLOCK_TD: {
         MD_BLOCK_TD_DETAIL *detail = static_cast<MD_BLOCK_TD_DETAIL *>(det);
@@ -299,6 +279,9 @@ int QTextMarkdownImporter::cbLeaveBlock(int blockType, void *detail)
 {
     Q_UNUSED(detail)
     switch (blockType) {
+    case MD_BLOCK_P:
+        m_listItem = false;
+        break;
     case MD_BLOCK_UL:
     case MD_BLOCK_OL:
         qCDebug(lcMD, "list at level %d ended", m_listStack.count());
@@ -525,19 +508,32 @@ int QTextMarkdownImporter::cbText(int textType, const char *text, unsigned size)
     return 0; // no error
 }
 
+/*!
+    Insert a new block based on stored state.
+
+    m_cursor cannot store the state for the _next_ block ahead of time, because
+    m_cursor->setBlockFormat() controls the format of the block that the cursor
+    is already in; so cbLeaveBlock() cannot call setBlockFormat() without
+    altering the block that was just added. Therefore cbLeaveBlock() and the
+    following cbEnterBlock() set variables to remember what formatting should
+    come next, and insertBlock() is called just before the actual text
+    insertion, to create a new block with the right formatting.
+*/
 void QTextMarkdownImporter::insertBlock()
 {
     QTextCharFormat charFormat;
     if (!m_spanFormatStack.isEmpty())
         charFormat = m_spanFormatStack.top();
     QTextBlockFormat blockFormat;
+    if (!m_listStack.isEmpty() && !m_needsInsertList && m_listItem) {
+        QTextList *list = m_listStack.top();
+        blockFormat = list->item(list->count() - 1).blockFormat();
+    }
     if (m_blockQuoteDepth) {
         blockFormat.setProperty(QTextFormat::BlockQuoteLevel, m_blockQuoteDepth);
         blockFormat.setLeftMargin(BlockQuoteIndent * m_blockQuoteDepth);
         blockFormat.setRightMargin(BlockQuoteIndent);
     }
-    if (m_listStack.count())
-        blockFormat.setIndent(m_listStack.count());
     if (m_codeBlock) {
         blockFormat.setProperty(QTextFormat::BlockCodeLanguage, m_blockCodeLanguage);
         charFormat.setFont(m_monoFont);
@@ -545,7 +541,19 @@ void QTextMarkdownImporter::insertBlock()
         blockFormat.setTopMargin(m_paragraphMargin);
         blockFormat.setBottomMargin(m_paragraphMargin);
     }
+    if (m_markerType == QTextBlockFormat::NoMarker)
+        blockFormat.clearProperty(QTextFormat::BlockMarker);
+    else
+        blockFormat.setMarker(m_markerType);
+    if (!m_listStack.isEmpty())
+        blockFormat.setIndent(m_listStack.count());
     m_cursor->insertBlock(blockFormat, charFormat);
+    if (m_needsInsertList) {
+        m_listStack.push(m_cursor->createList(m_listFormat));
+    } else if (!m_listStack.isEmpty() && m_listItem) {
+        m_listStack.top()->add(m_cursor->block());
+    }
+    m_needsInsertList = false;
     m_needsInsertBlock = false;
 }
 

src/gui/text/qtextmarkdownimporter_p.h

@@ -123,14 +123,15 @@ private:
     int m_tableRowCount = 0;
     int m_tableCol = -1; // because relative cell movements (e.g. m_cursor->movePosition(QTextCursor::NextCell)) don't work
     int m_paragraphMargin = 0;
-    Features m_features;
     int m_blockType = 0;
-    bool m_emptyList = false; // true when the last thing we did was insertList
-    bool m_listItem = false;
-    bool m_emptyListItem = false;
+    Features m_features;
+    QTextListFormat m_listFormat;
+    QTextBlockFormat::MarkerType m_markerType = QTextBlockFormat::NoMarker;
+    bool m_needsInsertBlock = false;
+    bool m_needsInsertList = false;
+    bool m_listItem = false; // true from the beginning of LI to the end of the first P
     bool m_codeBlock = false;
     bool m_imageSpan = false;
-    bool m_needsInsertBlock = false;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QTextMarkdownImporter::Features)

tests/auto/gui/text/qtextmarkdownwriter/data/headingsAndLists.md

+# heading 1
+
+- list item 1
+- list item 2
+
+## heading 2
+
+1)  list item 1
+2)  list item 2
+
+the end paragraph
+

tests/auto/gui/text/qtextmarkdownwriter/tst_qtextmarkdownwriter.cpp

@@ -357,6 +357,7 @@ void tst_QTextMarkdownWriter::rewriteDocument_data()
 
     QTest::newRow("block quotes") << "blockquotes.md";
     QTest::newRow("example") << "example.md";
+    QTest::newRow("list items after headings") << "headingsAndLists.md";
 }
 
 void tst_QTextMarkdownWriter::rewriteDocument()