Allow extensions to define the keys returned by linkcode ext #11824
Conversation
n-peugnet
force-pushed
the
linkcode-generic
branch
3 times, most recently
from
9fe04da
to
669a355
Compare
n-peugnet
force-pushed
the
linkcode-generic
branch
from
669a355
to
727787b
Compare
|
It appears to be quite similar to what we do for search languages so I'd say it's fine? Can you also add a CHANGELOG entry and a test? (probably of the same kind as those for the search languages, if any) |
|
@picnixz I just pushed commits that add a test and a changelog entry. Don't hesitate to edit it and my change in the docs as well if you find better ways to describe it. P.S. I will also add a small sentence in the linkcode docs that links to this new function. |
|
Windows tests for PO files are flaky, don't worry about it |
|
By the way, in the future, it's better not to force push commits. We squash them before merging and force-pushing makes review harder (I also used to rebase/force push) |
sphinx/application.py
Outdated
| .. versionadded:: 7.3 | ||
| """ | ||
| from sphinx.ext.linkcode import domain_keys | ||
| domain_keys[domain] = keys |
There was a problem hiding this comment.
Just wondering but what happens if two extensions do the same? should extend the keys and removing duplicates?
Also, you should probably say (in the doc) that the keys an extension defines overrides whatever keys were previously defined if we go with the current implementation.
There was a problem hiding this comment.
Just wondering but what happens if two extensions do the same? should extend the keys and removing duplicates?
I don't think so, the idea is that an extension that provides support for a new domain, for instance sphinxcontrib-phpdomain, can now add the php domain to the list of domains supported by linkcode. I don't think that anybody uses two extensions adding support for the same domain, so I don't see any chances of conflict here.
Another possibility I see would be to completely forbid overriding an existing key, or maybe add a warning when a key is overridden. But I thought overriding the existing keys could also be useful in a way.
Maybe we should wait for another opinion? I'm not in a hurry for this feature.
Also, you should probably say (in the doc) that the keys an extension defines overrides whatever keys were previously defined if we go with the current implementation.
I will add a more descriptive explanation when we settle on the preferred behaviour.
There was a problem hiding this comment.
I would like the opinion of another maintainer/contributor. Since we are in a holiday period, it might take a while though. When we release new versions, we create an issue and you can ask at that moment if we didn't decide on anything.
|
@jdknight you were looking at linkcode recently - any thoughts on this? |
Adding support for custom domains looks like a good idea to me. |
# Conflicts: # CHANGES.rst # sphinx/application.py # sphinx/ext/linkcode.py # tests/roots/test-ext-viewcode/conf.py # tests/test_extensions/test_ext_viewcode.py
|
@n-peugnet I am not sure that this should be a method on A |
If I remember correctly, the reason I chose to add it in So this is fine by me. |
Subject: Allow extensions to define the keys the linkcode extension should return for a domain.
Feature or Bugfix
Purpose
The linkcode extension currently only works for some pre-defined domains.
By allowing extensions to define the keys associated with a domain, it opens up the linkcode extension to a lot more new domains.
For instance, in the sphinxcontrib-phpdomain, one could simply add the following in
setup(app)function to make linkcode work for thephpdomain:Detail
Relates
P.S. Ah sorry, I made this commit on an old tag for my tests, I will rebase my branch.