Fix several issues regarding duplicate/empty xml:id

- Don't output an anchor in a table (except in cells). This is not valid DocBook and will likely never be: https://github.com/docbook/docbook/issues/233
- Output anchors only when their ID is nonempty. This behavior seldom happens when the original \target is empty, which is often a problem in the original file being documented.
- Avoid xml:id clash between a \target and a \section.
- Simplify the code: maintaining a pointer to the next atom is not relevant.
- Improve detection of empty table rows: some tables include keywords between a \row and the first \li, like qtdoc/qmlapp/debugging.qdoc.

Change-Id: Iece9a2236741e307995e357e3a2e7f674a16e66f
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
(cherry picked from commit d1ee606afacbc0b1677263f2d425c686d3233847)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

1 files changed, 49 insertions, 5 deletions

diff --git a/src/qdoc/docbookgenerator.cpp b/src/qdoc/docbookgenerator.cpp
index 351c43d95..efdfbc79f 100644
--- a/src/qdoc/docbookgenerator.cpp
+++ b/src/qdoc/docbookgenerator.cpp
@@ -93,6 +93,9 @@ void DocBookGenerator::endSection()
 
 void DocBookGenerator::writeAnchor(const QString &id)
 {
+    if (id.isEmpty())
+        return;
+
     m_writer->writeEmptyElement(dbNamespace, "anchor");
     writeXmlId(id);
     newLine();
@@ -836,17 +839,27 @@ qsizetype DocBookGenerator::generateAtom(const Atom *atom, const Node *relative)
         m_writer->writeEndElement(); // table
         newLine();
         break;
-    case Atom::TableHeaderLeft:
+    case Atom::TableHeaderLeft: {
         if (matchAhead(atom, Atom::TableHeaderRight)) {
-            skipAhead += 1;
+            ++skipAhead;
             break;
         }
 
+        const Atom *next = atom->next();
+        QString id{""};
+        if (matchAhead(atom, Atom::Target)) {
+            id = Doc::canonicalTitle(next->string());
+            next = next->next();
+            ++skipAhead;
+        }
+
         m_writer->writeStartElement(dbNamespace, "thead");
         newLine();
         m_writer->writeStartElement(dbNamespace, "tr");
+        writeXmlId(id);
         newLine();
         m_inTableHeader = true;
+    }
         break;
     case Atom::TableHeaderRight:
         m_writer->writeEndElement(); // tr
@@ -861,11 +874,21 @@ qsizetype DocBookGenerator::generateAtom(const Atom *atom, const Node *relative)
             m_inTableHeader = false;
         }
         break;
-    case Atom::TableRowLeft:
+    case Atom::TableRowLeft: {
         if (matchAhead(atom, Atom::TableRowRight))
             break;
 
+        QString id{""};
+        bool hasTarget {false};
+        if (matchAhead(atom, Atom::Target)) {
+            id = Doc::canonicalTitle(atom->next()->string());
+            ++skipAhead;
+            hasTarget = true;
+        }
+
         m_writer->writeStartElement(dbNamespace, "tr");
+        writeXmlId(id);
+
         if (atom->string().isEmpty()) {
             m_writer->writeAttribute("valign", "top");
         } else {
@@ -897,12 +920,24 @@ qsizetype DocBookGenerator::generateAtom(const Atom *atom, const Node *relative)
         }
         newLine();
 
-        if (!matchAhead(atom, Atom::TableItemLeft)) {
+        // If there is nothing in this row, close it right now. There might be
+        // keywords before the row contents.
+        bool isRowEmpty = hasTarget
+            ? !matchAhead(atom->next(), Atom::TableItemLeft)
+            : !matchAhead(atom, Atom::TableItemLeft);
+        if (isRowEmpty && matchAhead(atom, Atom::Keyword)) {
+            const Atom* next = atom->next();
+            while (matchAhead(next, Atom::Keyword))
+                next = next->next();
+            isRowEmpty = !matchAhead(next, Atom::TableItemLeft);
+        }
+
+        if (isRowEmpty) {
             closeTableRow = true;
             m_writer->writeEndElement(); // td
             newLine();
         }
-
+    }
         break;
     case Atom::TableRowRight:
         if (closeTableRow) {
@@ -944,6 +979,15 @@ qsizetype DocBookGenerator::generateAtom(const Atom *atom, const Node *relative)
     case Atom::Keyword:
         break;
     case Atom::Target:
+        // Sometimes, there is a \target just before a section title with
+        // the same ID. Only output one xml:id.
+        if (matchAhead(atom, Atom::SectionRight) && matchAhead(atom->next(), Atom::SectionLeft)) {
+            QString nextId = Doc::canonicalTitle(Text::sectionHeading(atom->next()->next()).toString());
+            QString ownId = Doc::canonicalTitle(atom->string());
+            if (nextId == ownId)
+                break;
+        }
+
         writeAnchor(Doc::canonicalTitle(atom->string()));
         break;
     case Atom::UnhandledFormat: