Duplicate labels in `.tex` output

[tim-nordell-nimbelink]

When building the LaTeX files, it complains about duplicate labels. I've created a test case against this over here: https://github.com/tim-nordell-nimbelink/sphinx-needs/tree/test_for_duplicate_label. (No fix currently.)

The error seems innocuous - that is, the build still completes, and the duplicate label is close to the original label in the resulting document so it's not causing a link to go to a weird part of the document.

[tim-nordell-nimbelink]

I will note that this is introduced in master and not present in 1.0.2. It's possible this is related to #796.

[tim-nordell-nimbelink]

Bisected to e51fc1fcf398de5c06a519b2281189051baf0e85.

[tim-nordell-nimbelink]

From the LaTeX builder:

        if node.get('refid'):
            prev_node = get_prev_node(node)
            ...
                add_target(node['refid'])
        for id in node['ids']:
            add_target(id)

It looks like both the attributes ids and refid are considered for creating LaTeX targets. Commit e51fc1f introduced a new layer to the output (a <container> element); I'm not sure why doing that caused LaTeX to emit multiple hyperrefs, and why without it it squashed those attributes basically. It's weird. Reducing the usage of those attributes "fixes" the LaTeX output, but I don't know enough about why the need ID was repeated multiple times.

The following patch fixes the LaTeX build complains (I haven't tested actual rendering and following of links yet with this patch):

diff --git a/sphinx_needs/api/need.py b/sphinx_needs/api/need.py
index b7d6d6d..57ef257 100644
--- a/sphinx_needs/api/need.py
+++ b/sphinx_needs/api/need.py
@@ -198,7 +198,7 @@ def add_need(
     if is_external:
         target_node = None
     else:
-        target_node = nodes.target("", "", ids=[need_id], refid=need_id)
+        target_node = nodes.target("", "", refid=need_id)
         external_url = None
 
     # Handle status
diff --git a/sphinx_needs/layout.py b/sphinx_needs/layout.py
index a9225e4..9ff892c 100644
--- a/sphinx_needs/layout.py
+++ b/sphinx_needs/layout.py
@@ -185,7 +185,7 @@ class LayoutHandler:
 
         classes.append("needs_type_" + "".join(self.need["type"].split()))
 
-        self.node_table = nodes.table(classes=classes, ids=[self.need["id"]])
+        self.node_table = nodes.table(classes=classes)
         self.node_tbody = nodes.tbody()
 
         self.grids = {

[danwos]

Thanks for the patch 👍

Can you create a PR for this, so that our CI can already test it?

We may need the id for some JS actions. I will check this when I have some free minutes.

[tim-nordell-nimbelink]

@danwos Sure thing! I just wasn't sure when/where those ids were needed and didn't want to break things, so I figured it was better for discussion here first. I'll create a PR.

[tim-nordell-nimbelink]

I just checked the html output, and there's a nesting like this for a need:

<div ... id="SNCB-....">
<table ... id="R_XXXX">

With this patch, the id= is gone from the <table> element only. The href is still in the output.

[tim-nordell-nimbelink]

It breaks the HTML anchors actually removing that id. I wonder if it can be moved to the div instead (checking).

[tim-nordell-nimbelink]

Looks like if I add it into the container, it generates a span, so it looks like:

<div ... id="SNCB-...">
<span id="R_XXXX"></span>
<table>

which works for both the HTML case, and the LaTeX case. Win win! Commit is updated with this change.

[danwos]

@iSOLveIT do you see some problems with the Collapse-solution from yesterday if the HTML output is changed like this?

[iSOLveIT]

Hi @tim-nordell-nimbelink and @danwos,

The change shouldn't affect the collapse button if the table element still has the attribute, class=”need”. But the draft PR for this issue (#814) doesn't pass the CI and I can't also build locally. Here is the error I get:

Traceback (most recent call last):
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/events.py", line 94, in emit
   results.append(listener.handler(self.app, *args))
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/sphinx_needs/needs.py", line 430, in process_caller
   check_func(app, doctree, fromdocname, current_nodes[check_node])
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/sphinx_needs/directives/needuml.py", line 395, in process_needuml
   parent_need_id = is_element_of_need(node)
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/sphinx_needs/directives/needuml.py", line 373, in is_element_of_need
   is_element_of = node["ids"][0]
IndexError: list index out of range

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/cmd/build.py", line 279, in build_main
   app.build(args.force_all, filenames)
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/application.py", line 350, in build
   self.builder.build_update()
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 302, in build_update
   self.build(to_build,
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 368, in build
   self.write(docnames, list(updated_docnames), method)
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 563, in write
   self._write_serial(sorted(docnames))
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 570, in _write_serial
   doctree = self.env.get_and_resolve_doctree(docname, self)
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/environment/__init__.py", line 591, in get_and_resolve_doctree
   self.apply_post_transforms(doctree, docname)
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/environment/__init__.py", line 642, in apply_post_transforms
   self.events.emit('doctree-resolved', doctree, docname)
 File "/home/isolveit/Documents/myCodes/Codes/useBlocks/sphinx-needs/.venv/lib/python3.9/site-packages/sphinx/events.py", line 105, in emit
   raise ExtensionError(__("Handler %r for event %r threw an exception") %
sphinx.errors.ExtensionError: Handler <function process_creator.<locals>.process_caller at 0x7f85c680c4c0> for event 'doctree-resolved' threw an exception (exception: list index out of range)

Extension error (sphinx_needs.needs):
Handler <function process_creator.<locals>.process_caller at 0x7f85c680c4c0> for event 'doctree-resolved' threw an exception (exception: list index out of range)

[iSOLveIT]

I figured out the issue and a solution for it. The PR (#814) by @tim-nordell-nimbelink must make some changes to the sphinx_needs/directives/needuml.py file at line 373. Since we remove the id attribute from the <table> element, the code on line 373, is_element_of = node["ids"][0], will result in an error (IndexError: list index out of range).

Solution: The fix is to change line 373 to is_element_of = node.parent.attributes["ids"][1]. We are now taking the id from the span element:

<div ... id="SNCB-...">
<span id="R_XXXX"></span>  # id taken from here.
<table>

[danwos]

I'm not sure if I like this much, as the table is the need element and it should have the related need ID. Technically it is no problem to get it from a <span> element, but it makes not much sense, as <span> is empty.

With CSS you may also want to address the table element of a given need and not the <span>.

Maybe it's possible to keep the ID for the table and remove it from other places? Or use different IDs, so that there is no conflict in latex?

[tim-nordell-nimbelink]

From the code, the references for SNCB:

layout.py:    container_id = "SNCB-" + str(uuid.uuid4())[:8]
libs/html/sphinx_needs_collapse.js:        var need_table_id = table.closest("div[id^=SNCB-]").attr("id");
libs/html/sphinx_needs_collapse.js:            var need_table_id = table.closest("div[id^=SNCB-]").attr("id");

@danwos @iSOLveIT What are the SNCB-* links used for? Could those reference the need ID instead if the need ID was brought out to here? E.g. find the ID of the closest need_container? It looks like these just need a unique identifier here, and the need ID by definition is unique. If that's the case, the need ID could be moved into that outer div.

I don't know Javascript/HTML/CSS all that well, but could the need_table_id search for the class need_container instead of SNCB-*? They coincide in the same HTML element.

[tim-nordell-nimbelink]

I pushed a couple of fixup commits (can be squashed into the main commit) showing those transforms (along with the fix for the ID reference).

[iSOLveIT]

What are the SNCB-* links used for? Could those reference the need ID instead if the need ID was brought out to here? E.g. find the ID of the closest need_container? It looks like these just need a unique identifier here, and the need ID by definition is unique. If that's the case, the need ID could be moved into that outer div.

The need ID by definition is unique but sometimes you can have 1 or more need tables having the same IDs (eg. when you use needextract directive. That is why we have the SNCB-* links.

I don't know Javascript/HTML/CSS all that well, but could the need_table_id search for the class need_container instead of SNCB-*? They coincide in the same HTML element.

Using the need_container class will cause a malfunction in the collapse button.

[tim-nordell-nimbelink]

Looking more closely at the LaTeX writer's internal logic, I did find another way to mask the LaTeX issue:

--- a/sphinx_needs/api/need.py
+++ b/sphinx_needs/api/need.py
@@ -197,7 +197,7 @@ def add_need(
     if is_external:
         target_node = None
     else:
-        target_node = nodes.target("", "", ids=[need_id], refid=need_id)
+        target_node = nodes.target("", "", ids=[need_id], refid=need_id, anonymous="")
         external_url = None
 
     # Handle status

I don't know what the intent behind the anonymous property is for, but the LaTeX writer looks for it. Here's the larger context around it:

    def visit_target(self, node: Element) -> None:
        ...
        if isinstance(next_node, HYPERLINK_SUPPORT_NODES):
            return
        elif domain.get_enumerable_node_type(next_node) and domain.get_numfig_title(next_node):
            return

        if 'refuri' in node:
            return
        if 'anonymous' in node:
            return
        if node.get('refid'):
            prev_node = get_prev_node(node)
            if isinstance(prev_node, nodes.reference) and node['refid'] == prev_node['refid']:
                # a target for a hyperlink reference having alias
                pass
            else:
                add_target(node['refid'])
        for id in node['ids']:
            add_target(id)

The code was added in reference to https://github.com/sphinx-doc/sphinx/issues/7701.

Testing against needextract, this still generates duplicate label complains in LaTeX, but this is probably expected.

[iSOLveIT]

Hi @tim-nordell-nimbelink I hope you are doing well. I would appreciate it if we could meet on Google Meet to discuss about this issue. I would need some clarifications to help with providing a solution that works for both latex and HTML builds. Kindly let me know if the meeting could be possible and when you are free to do so.

Thank you.

[tim-nordell-nimbelink]

Hi @iSOLveIT -

Basically any label/reference should be only once in the outbound .tex file. You can find the \label{\detokenize{...}} inside the .tex output from a project. There should only be one instance of a given label across the entire document, or LaTeX has a warning. I'm not sure which label LaTeX will put the hyperlink to inside the PDF document that is generated. The current patch, adding the anonymous property, causes the LaTeX output to render without issue aside from usage of needextract.

For needextract (I'm not currently using it, but just was noting that this looks like it regenerates the label), it completely replicates a given need, including the IDs. This probably works well for the HTML as long as the needextract is only on a single page (e.g. not the monolithic singlehtml build), but it's probably an issue for the singlehtml build, too.

Thanks, Tim

[iSOLveIT]

@tim-nordell-nimbelink Thanks for feedback. I would like to know the commands you use for the LaTeX build. Then any configurations you use when building.

[tim-nordell-nimbelink]

I'm using the latexpdf build (e.g. sphinx-build -M latexpdf source build - but it's observable just with sphinx-build -M latex ...) along with the default need layout of clean.

[iSOLveIT]

Ok, thank you very much.

[tim-nordell-nimbelink]

Yep, no problem! I'll note I added a test case for it in PR https://github.com/useblocks/sphinx-needs/pull/814, too, and the potential fix from above.

Edit: The test case doesn't cover the duplicate from needextract.