Skip to content

feat: external redirection#68

Open
jahn-junior wants to merge 2 commits into
sphinx-doc:0.xfrom
jahn-junior:work/external-redirects
Open

feat: external redirection#68
jahn-junior wants to merge 2 commits into
sphinx-doc:0.xfrom
jahn-junior:work/external-redirects

Conversation

@jahn-junior
Copy link
Copy Markdown

Add support for external redirects. Resolves #41.

The implementation involves a build_internal_redirect helper function that gets called in the main build_redirects loop if the destination string starts with http:// or https://

Open to any feedback on the overall design and implementation. Thanks in advance for your time!

tests: add basic test for external redirects
Comment thread sphinxext/rediraffe.py
Comment on lines +142 to +144
def is_external(dest: str) -> bool:
"""Return True if the redirect destination is a URL."""
return dest.startswith(('http://', 'https://'))
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is a bit shaky. Open to alternatives.

Comment thread tests/test_ext.py
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ensure_redirect wasn't built with external redirects in mind, so I figured it was better to stray from the pattern than to rewrite it for this one test.

@jahn-junior
Copy link
Copy Markdown
Author

@AA-Turner @TheTripleV Hello! I'm hoping to get some more opinions on this and #67. These two PRs unblock my team's adoption of the extension. Thanks for your time!

(Sorry if you aren't the right people to ping. I made a guess based on the commit history.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for rediracting to external urls

1 participant