Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #7276: cpp: objects generate hypertarget names unexpectedly #7277

Merged
merged 1 commit into from Mar 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES
Expand Up @@ -26,6 +26,8 @@ Incompatible changes
* #6903: Internal data structure of Python, reST and standard domains have
changed. The node_id is added to the index of objects and modules. Now they
contains a pair of docname and node_id for cross reference.
* #7276: C++ domain: Non intended behavior is removed such as ``say_hello_``
links to ``.. cpp:function:: say_hello()``
* #7210: js domain: Non intended behavior is removed such as ``parseInt_`` links
to ``.. js:function:: parseInt``
* #7229: rst domain: Non intended behavior is removed such as ``numref_`` links
Expand Down
4 changes: 0 additions & 4 deletions sphinx/domains/cpp.py
Expand Up @@ -6512,10 +6512,6 @@ def add_target_and_index(self, ast: ASTDeclaration, sig: str, signode: desc_sign
names = self.env.domaindata['cpp']['names']
if name not in names:
names[name] = ast.symbol.docname
signode['names'].append(name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I can reproduce the warning this is supposed to fix, but I don't actually know where it comes from. Do you know where it is generated from? (perhaps inside docutils?) In short: besides avoiding the warning I'm actually not sure what the specific effect of removing this line is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the warning came from inside docutils.

In short: besides avoiding the warning I'm actually not sure what the specific effect of removing this line is.

In docutils world, node['names'] is used to create a hyperlink target that is able to refer via its name. On the other hand, Sphinx provides some roles (ex. :cpp:func:) to realize the cross-reference feature because docutils does not support a reference between multiple documents. Therefore, it is not needed to generate names for the hyperlink target.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to merge.

else:
# print("[CPP] non-unique name:", name)
pass
# always add the newest id
assert newestId
signode['ids'].append(newestId)
Expand Down