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

Close #10125: extlinks: Improve suggestion message for a reference having title #10131

Merged
merged 1 commit into from Jan 30, 2022
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
1 change: 1 addition & 0 deletions CHANGES
Expand Up @@ -15,6 +15,7 @@ Features added

* #9494, #9456: html search: Add a config variable
:confval:`html_show_search_summary` to enable/disable the search summaries
* #10125: extlinks: Improve suggestion message for a reference having title

Bugs fixed
----------
Expand Down
9 changes: 7 additions & 2 deletions sphinx/ext/extlinks.py
Expand Up @@ -38,7 +38,7 @@
from sphinx.deprecation import RemovedInSphinx60Warning
from sphinx.locale import __
from sphinx.transforms.post_transforms import SphinxPostTransform
from sphinx.util import logging
from sphinx.util import logging, rst
from sphinx.util.nodes import split_explicit_title
from sphinx.util.typing import RoleFunction

Expand Down Expand Up @@ -67,6 +67,7 @@ def check_uri(self, refnode: nodes.reference) -> None:
return

uri = refnode['refuri']
title = refnode.astext()

for alias, (base_uri, _caption) in self.app.config.extlinks.items():
uri_pattern = re.compile(base_uri.replace('%s', '(?P<value>.+)'))
Expand All @@ -75,7 +76,11 @@ def check_uri(self, refnode: nodes.reference) -> None:
# build a replacement suggestion
msg = __('hardcoded link %r could be replaced by an extlink '
'(try using %r instead)')
replacement = f":{alias}:`{match.groupdict().get('value')}`"
value = match.groupdict().get('value')
if uri != title:
replacement = f":{alias}:`{rst.escape(title)} <{value}>`"
else:
replacement = f":{alias}:`{value}`"
logger.warning(msg, uri, replacement, location=refnode)


Expand Down
31 changes: 16 additions & 15 deletions tests/test_ext_extlinks.py
Expand Up @@ -5,14 +5,15 @@
def test_replaceable_uris_emit_extlinks_warnings(app, warning):
app.build()
warning_output = warning.getvalue()

# there should be exactly three warnings for replaceable URLs
message = (
"WARNING: hardcoded link 'https://github.com/sphinx-doc/sphinx/issues/1' "
"could be replaced by an extlink (try using ':issue:`1`' instead)"
"index.rst:%d: WARNING: hardcoded link 'https://github.com/sphinx-doc/sphinx/issues/1' "
"could be replaced by an extlink (try using '%s' instead)"
)
assert f"index.rst:11: {message}" in warning_output
assert f"index.rst:13: {message}" in warning_output
assert f"index.rst:15: {message}" in warning_output
assert message % (11, ":issue:`1`") in warning_output
assert message % (13, ":issue:`inline replaceable link <1>`") in warning_output
assert message % (15, ":issue:`replaceable link <1>`") in warning_output


@pytest.mark.sphinx('html', testroot='ext-extlinks-hardcoded-urls-multiple-replacements')
Expand All @@ -21,16 +22,16 @@ def test_all_replacements_suggested_if_multiple_replacements_possible(app, warni
warning_output = warning.getvalue()
# there should be six warnings for replaceable URLs, three pairs per link
message = (
"WARNING: hardcoded link 'https://github.com/octocat' "
"could be replaced by an extlink (try using ':user:`octocat`' instead)"
"index.rst:%d: WARNING: hardcoded link 'https://github.com/octocat' "
"could be replaced by an extlink (try using '%s' instead)"
)
assert f"index.rst:14: {message}" in warning_output
assert f"index.rst:16: {message}" in warning_output
assert f"index.rst:18: {message}" in warning_output
assert message % (14, ":user:`octocat`") in warning_output
assert message % (16, ":user:`inline replaceable link <octocat>`") in warning_output
assert message % (18, ":user:`replaceable link <octocat>`") in warning_output
message = (
"WARNING: hardcoded link 'https://github.com/octocat' "
"could be replaced by an extlink (try using ':repo:`octocat`' instead)"
"index.rst:%d: WARNING: hardcoded link 'https://github.com/octocat' "
"could be replaced by an extlink (try using '%s' instead)"
)
assert f"index.rst:14: {message}" in warning_output
assert f"index.rst:16: {message}" in warning_output
assert f"index.rst:18: {message}" in warning_output
assert message % (14, ":repo:`octocat`") in warning_output
assert message % (16, ":repo:`inline replaceable link <octocat>`") in warning_output
assert message % (18, ":repo:`replaceable link <octocat>`") in warning_output