From 801b8304fbbe09b3c05c4adf5c14a76f05daafcb Mon Sep 17 00:00:00 2001 From: Shawn Rutledge Date: Mon, 22 May 2023 14:20:12 +0200 Subject: [PATCH] Add QTextDocument* constructor argument to QTextMarkdownImporter QTextMarkdownImporter::import() took a QTD* as if it would be ok to reuse one instance of QTextMarkdownImporter for repeated importing into different documents; but in practice, we never do that: in fact it's usually a short-lived, stack-allocated object, as in QTextMarkdownImporter(&doc, QTMI::DialectGitHub).import(input); So it's less clumsy internally to require the document be provided to the constructor: that way a QTextCursor can be constructed immediately too, as part of the importer object rather than separately on the heap. This is private API, unused outside qtbase. Change-Id: I8041ceb33cb7e7608df55dc5a963292c585afb90 Reviewed-by: Oliver Eftevaag Reviewed-by: Giuseppe D'Angelo --- src/gui/text/qtextdocument.cpp | 2 +- src/gui/text/qtextdocumentfragment.cpp | 3 +- src/gui/text/qtextmarkdownimporter.cpp | 107 +++++++++--------- src/gui/text/qtextmarkdownimporter_p.h | 9 +- .../tst_qtextmarkdownimporter.cpp | 8 +- 5 files changed, 63 insertions(+), 66 deletions(-) diff --git a/src/gui/text/qtextdocument.cpp b/src/gui/text/qtextdocument.cpp index 88cef5f88e..151798b2e6 100644 --- a/src/gui/text/qtextdocument.cpp +++ b/src/gui/text/qtextdocument.cpp @@ -3554,7 +3554,7 @@ QString QTextDocument::toMarkdown(QTextDocument::MarkdownFeatures features) cons #if QT_CONFIG(textmarkdownreader) void QTextDocument::setMarkdown(const QString &markdown, QTextDocument::MarkdownFeatures features) { - QTextMarkdownImporter(features).import(this, markdown); + QTextMarkdownImporter(this, features).import(markdown); } #endif diff --git a/src/gui/text/qtextdocumentfragment.cpp b/src/gui/text/qtextdocumentfragment.cpp index 9e99992929..03d2afc0d1 100644 --- a/src/gui/text/qtextdocumentfragment.cpp +++ b/src/gui/text/qtextdocumentfragment.cpp @@ -1313,8 +1313,7 @@ QTextDocumentFragment QTextDocumentFragment::fromMarkdown(const QString &markdow QTextDocumentFragment res; res.d = new QTextDocumentFragmentPrivate; - QTextMarkdownImporter importer(features); - importer.import(res.d->doc, markdown); + QTextMarkdownImporter(res.d->doc, features).import(markdown); return res; } diff --git a/src/gui/text/qtextmarkdownimporter.cpp b/src/gui/text/qtextmarkdownimporter.cpp index b43e1583cf..1dfa1e6652 100644 --- a/src/gui/text/qtextmarkdownimporter.cpp +++ b/src/gui/text/qtextmarkdownimporter.cpp @@ -104,18 +104,19 @@ static Qt::Alignment MdAlignment(MD_ALIGN a, Qt::Alignment defaultAlignment = Qt } } -QTextMarkdownImporter::QTextMarkdownImporter(QTextMarkdownImporter::Features features) - : m_monoFont(QFontDatabase::systemFont(QFontDatabase::FixedFont)) +QTextMarkdownImporter::QTextMarkdownImporter(QTextDocument *doc, QTextMarkdownImporter::Features features) + : m_cursor(doc) + , m_monoFont(QFontDatabase::systemFont(QFontDatabase::FixedFont)) , m_features(features) { } -QTextMarkdownImporter::QTextMarkdownImporter(QTextDocument::MarkdownFeatures features) - : QTextMarkdownImporter(static_cast(int(features))) +QTextMarkdownImporter::QTextMarkdownImporter(QTextDocument *doc, QTextDocument::MarkdownFeatures features) + : QTextMarkdownImporter(doc, static_cast(int(features))) { } -void QTextMarkdownImporter::import(QTextDocument *doc, const QString &markdown) +void QTextMarkdownImporter::import(const QString &markdown) { MD_PARSER callbacks = { 0, // abi_version @@ -128,21 +129,19 @@ void QTextMarkdownImporter::import(QTextDocument *doc, const QString &markdown) &CbDebugLog, nullptr // syntax }; - m_doc = doc; - m_paragraphMargin = m_doc->defaultFont().pointSize() * 2 / 3; - m_cursor = new QTextCursor(doc); + QTextDocument *doc = m_cursor.document(); + const auto defaultFont = doc->defaultFont(); + m_paragraphMargin = defaultFont.pointSize() * 2 / 3; doc->clear(); - if (doc->defaultFont().pointSize() != -1) - m_monoFont.setPointSize(doc->defaultFont().pointSize()); + if (defaultFont.pointSize() != -1) + m_monoFont.setPointSize(defaultFont.pointSize()); else - m_monoFont.setPixelSize(doc->defaultFont().pixelSize()); - qCDebug(lcMD) << "default font" << doc->defaultFont() << "mono font" << m_monoFont; + m_monoFont.setPixelSize(defaultFont.pixelSize()); + qCDebug(lcMD) << "default font" << defaultFont << "mono font" << m_monoFont; QByteArray md = markdown.toUtf8(); - m_cursor->beginEditBlock(); + m_cursor.beginEditBlock(); md_parse(md.constData(), MD_SIZE(md.size()), &callbacks, this); - m_cursor->endEditBlock(); - delete m_cursor; - m_cursor = nullptr; + m_cursor.endEditBlock(); } int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det) @@ -181,11 +180,11 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det) charFmt.setFontWeight(QFont::Bold); blockFmt.setHeadingLevel(int(detail->level)); m_needsInsertBlock = false; - if (m_doc->isEmpty()) { - m_cursor->setBlockFormat(blockFmt); - m_cursor->setCharFormat(charFmt); + if (m_cursor.document()->isEmpty()) { + m_cursor.setBlockFormat(blockFmt); + m_cursor.setCharFormat(charFmt); } else { - m_cursor->insertBlock(blockFmt, charFmt); + m_cursor.insertBlock(blockFmt, charFmt); } qCDebug(lcMD, "H%d", detail->level); } break; @@ -200,7 +199,7 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det) } break; case MD_BLOCK_UL: { if (m_needsInsertList) // list nested in an empty list - m_listStack.push(m_cursor->insertList(m_listFormat)); + m_listStack.push(m_cursor.insertList(m_listFormat)); else m_needsInsertList = true; MD_BLOCK_UL_DETAIL *detail = static_cast(det); @@ -221,7 +220,7 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det) } break; case MD_BLOCK_OL: { if (m_needsInsertList) // list nested in an empty list - m_listStack.push(m_cursor->insertList(m_listFormat)); + m_listStack.push(m_cursor.insertList(m_listFormat)); else m_needsInsertList = true; MD_BLOCK_OL_DETAIL *detail = static_cast(det); @@ -242,10 +241,10 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det) qWarning("malformed table in Markdown input"); return 1; } - *m_cursor = cell.firstCursorPosition(); - QTextBlockFormat blockFmt = m_cursor->blockFormat(); + m_cursor = cell.firstCursorPosition(); + QTextBlockFormat blockFmt = m_cursor.blockFormat(); blockFmt.setAlignment(MdAlignment(detail->align)); - m_cursor->setBlockFormat(blockFmt); + m_cursor.setBlockFormat(blockFmt); qCDebug(lcMD) << "TD; align" << detail->align << MdAlignment(detail->align) << "col" << m_tableCol; } break; case MD_BLOCK_TH: { @@ -273,13 +272,13 @@ int QTextMarkdownImporter::cbEnterBlock(int blockType, void *det) case MD_BLOCK_TABLE: m_tableColumnCount = 0; m_tableRowCount = 0; - m_currentTable = m_cursor->insertTable(1, 1); // we don't know the dimensions yet + m_currentTable = m_cursor.insertTable(1, 1); // we don't know the dimensions yet break; case MD_BLOCK_HR: { qCDebug(lcMD, "HR"); QTextBlockFormat blockFmt; blockFmt.setProperty(QTextFormat::BlockTrailingHorizontalRulerWidth, 1); - m_cursor->insertBlock(blockFmt, QTextCharFormat()); + m_cursor.insertBlock(blockFmt, QTextCharFormat()); } break; default: break; // nothing to do for now @@ -297,7 +296,7 @@ int QTextMarkdownImporter::cbLeaveBlock(int blockType, void *detail) case MD_BLOCK_UL: case MD_BLOCK_OL: if (Q_UNLIKELY(m_needsInsertList)) - m_listStack.push(m_cursor->createList(m_listFormat)); + m_listStack.push(m_cursor.createList(m_listFormat)); if (Q_UNLIKELY(m_listStack.isEmpty())) { qCWarning(lcMD, "list ended unexpectedly"); } else { @@ -335,7 +334,7 @@ int QTextMarkdownImporter::cbLeaveBlock(int blockType, void *detail) case MD_BLOCK_TABLE: qCDebug(lcMD) << "table ended with" << m_currentTable->columns() << "cols and" << m_currentTable->rows() << "rows"; m_currentTable = nullptr; - m_cursor->movePosition(QTextCursor::End); + m_cursor.movePosition(QTextCursor::End); break; case MD_BLOCK_LI: qCDebug(lcMD, "LI at level %d ended", int(m_listStack.size())); @@ -352,7 +351,7 @@ int QTextMarkdownImporter::cbLeaveBlock(int blockType, void *detail) m_needsInsertBlock = true; } break; case MD_BLOCK_H: - m_cursor->setCharFormat(QTextCharFormat()); + m_cursor.setCharFormat(QTextCharFormat()); break; default: break; @@ -406,7 +405,7 @@ int QTextMarkdownImporter::cbEnterSpan(int spanType, void *det) qCDebug(lcMD) << spanType << "setCharFormat" << charFmt.font().families().first() << charFmt.fontWeight() << (charFmt.fontItalic() ? "italic" : "") << charFmt.foreground().color().name(); - m_cursor->setCharFormat(charFmt); + m_cursor.setCharFormat(charFmt); return 0; // no error } @@ -419,7 +418,7 @@ int QTextMarkdownImporter::cbLeaveSpan(int spanType, void *detail) if (!m_spanFormatStack.isEmpty()) charFmt = m_spanFormatStack.top(); } - m_cursor->setCharFormat(charFmt); + m_cursor.setCharFormat(charFmt); qCDebug(lcMD) << spanType << "setCharFormat" << charFmt.font().families().first() << charFmt.fontWeight() << (charFmt.fontItalic() ? "italic" : "") << charFmt.foreground().color().name(); @@ -464,7 +463,7 @@ int QTextMarkdownImporter::cbText(int textType, const char *text, unsigned size) if (m_htmlTagDepth) m_htmlAccumulator += s; else - m_cursor->insertHtml(s); + m_cursor.insertHtml(s); s = QString(); break; #endif @@ -486,11 +485,11 @@ int QTextMarkdownImporter::cbText(int textType, const char *text, unsigned size) m_htmlAccumulator += s; if (!m_htmlTagDepth) { // all open tags are now closed qCDebug(lcMD) << "HTML" << m_htmlAccumulator; - m_cursor->insertHtml(m_htmlAccumulator); + m_cursor.insertHtml(m_htmlAccumulator); if (m_spanFormatStack.isEmpty()) - m_cursor->setCharFormat(QTextCharFormat()); + m_cursor.setCharFormat(QTextCharFormat()); else - m_cursor->setCharFormat(m_spanFormatStack.top()); + m_cursor.setCharFormat(m_spanFormatStack.top()); m_htmlAccumulator = QString(); } #endif @@ -520,24 +519,24 @@ int QTextMarkdownImporter::cbText(int textType, const char *text, unsigned size) m_imageFormat.setProperty(QTextFormat::ImageAltText, s); qCDebug(lcMD) << "image" << m_imageFormat.name() << "title" << m_imageFormat.stringProperty(QTextFormat::ImageTitle) - << "alt" << s << "relative to" << m_doc->baseUrl(); - m_cursor->insertImage(m_imageFormat); + << "alt" << s << "relative to" << m_cursor.document()->baseUrl(); + m_cursor.insertImage(m_imageFormat); return 0; // no error } if (!s.isEmpty()) - m_cursor->insertText(s); - if (m_cursor->currentList()) { + m_cursor.insertText(s); + if (m_cursor.currentList()) { // The list item will indent the list item's text, so we don't need indentation on the block. - QTextBlockFormat bfmt = m_cursor->blockFormat(); + QTextBlockFormat bfmt = m_cursor.blockFormat(); bfmt.setIndent(0); - m_cursor->setBlockFormat(bfmt); + m_cursor.setBlockFormat(bfmt); } if (lcMD().isEnabled(QtDebugMsg)) { - QTextBlockFormat bfmt = m_cursor->blockFormat(); + QTextBlockFormat bfmt = m_cursor.blockFormat(); QString debugInfo; - if (m_cursor->currentList()) - debugInfo = "in list at depth "_L1 + QString::number(m_cursor->currentList()->format().indent()); + if (m_cursor.currentList()) + debugInfo = "in list at depth "_L1 + QString::number(m_cursor.currentList()->format().indent()); if (bfmt.hasProperty(QTextFormat::BlockQuoteLevel)) debugInfo += "in blockquote at depth "_L1 + QString::number(bfmt.intProperty(QTextFormat::BlockQuoteLevel)); @@ -554,7 +553,7 @@ int QTextMarkdownImporter::cbText(int textType, const char *text, unsigned size) 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 + 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 @@ -596,19 +595,19 @@ void QTextMarkdownImporter::insertBlock() blockFormat.setMarker(m_markerType); if (!m_listStack.isEmpty()) blockFormat.setIndent(m_listStack.size()); - if (m_doc->isEmpty()) { - m_cursor->setBlockFormat(blockFormat); - m_cursor->setCharFormat(charFormat); + if (m_cursor.document()->isEmpty()) { + m_cursor.setBlockFormat(blockFormat); + m_cursor.setCharFormat(charFormat); } else if (m_listItem) { - m_cursor->insertBlock(blockFormat, QTextCharFormat()); - m_cursor->setCharFormat(charFormat); + m_cursor.insertBlock(blockFormat, QTextCharFormat()); + m_cursor.setCharFormat(charFormat); } else { - m_cursor->insertBlock(blockFormat, charFormat); + m_cursor.insertBlock(blockFormat, charFormat); } if (m_needsInsertList) { - m_listStack.push(m_cursor->createList(m_listFormat)); + m_listStack.push(m_cursor.createList(m_listFormat)); } else if (!m_listStack.isEmpty() && m_listItem && m_listStack.top()) { - m_listStack.top()->add(m_cursor->block()); + m_listStack.top()->add(m_cursor.block()); } m_needsInsertList = false; m_needsInsertBlock = false; diff --git a/src/gui/text/qtextmarkdownimporter_p.h b/src/gui/text/qtextmarkdownimporter_p.h index b7974da56e..89e0ab4ad1 100644 --- a/src/gui/text/qtextmarkdownimporter_p.h +++ b/src/gui/text/qtextmarkdownimporter_p.h @@ -55,10 +55,10 @@ public: }; Q_DECLARE_FLAGS(Features, Feature) - QTextMarkdownImporter(Features features); - QTextMarkdownImporter(QTextDocument::MarkdownFeatures features); + QTextMarkdownImporter(QTextDocument *doc, Features features); + QTextMarkdownImporter(QTextDocument *doc, QTextDocument::MarkdownFeatures features); - void import(QTextDocument *doc, const QString &markdown); + void import(const QString &markdown); public: // MD4C callbacks @@ -72,8 +72,7 @@ private: void insertBlock(); private: - QTextDocument *m_doc = nullptr; - QTextCursor *m_cursor = nullptr; + QTextCursor m_cursor; QTextTable *m_currentTable = nullptr; // because m_cursor->currentTable() doesn't work #if QT_CONFIG(regularexpression) QString m_htmlAccumulator; diff --git a/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp b/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp index a9b519f2d7..0fd3a7c22d 100644 --- a/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp +++ b/tests/auto/gui/text/qtextmarkdownimporter/tst_qtextmarkdownimporter.cpp @@ -101,7 +101,7 @@ void tst_QTextMarkdownImporter::headingBulletsContinuations() f.close(); QTextDocument doc; - QTextMarkdownImporter(QTextMarkdownImporter::DialectGitHub).import(&doc, md); + QTextMarkdownImporter(&doc, QTextMarkdownImporter::DialectGitHub).import(md); QTextFrame::iterator iterator = doc.rootFrame()->begin(); QTextFrame *currentFrame = iterator.currentFrame(); QStringList::const_iterator expectedIt = expectedBlocks.constBegin(); @@ -149,7 +149,7 @@ void tst_QTextMarkdownImporter::thematicBreaks() f.close(); QTextDocument doc; - QTextMarkdownImporter(QTextMarkdownImporter::DialectGitHub).import(&doc, md); + QTextMarkdownImporter(&doc, QTextMarkdownImporter::DialectGitHub).import(md); QTextFrame::iterator iterator = doc.rootFrame()->begin(); QTextFrame *currentFrame = iterator.currentFrame(); int i = 0; @@ -423,7 +423,7 @@ void tst_QTextMarkdownImporter::avoidBlankLineAtBeginning() // QTBUG-81060 QFETCH(int, expectedNumberOfParagraphs); QTextDocument doc; - QTextMarkdownImporter(QTextMarkdownImporter::DialectGitHub).import(&doc, input); + QTextMarkdownImporter(&doc, QTextMarkdownImporter::DialectGitHub).import(input); QTextFrame::iterator iterator = doc.rootFrame()->begin(); int i = 0; while (!iterator.atEnd()) { @@ -468,7 +468,7 @@ void tst_QTextMarkdownImporter::fragmentsAndProperties() QFETCH(int, expectedNumberOfFragments); QTextDocument doc; - QTextMarkdownImporter(QTextMarkdownImporter::DialectGitHub).import(&doc, input); + QTextMarkdownImporter(&doc, QTextMarkdownImporter::DialectGitHub).import(input); #ifdef DEBUG_WRITE_HTML { QFile out("/tmp/" + QLatin1String(QTest::currentDataTag()) + ".html");