-
Notifications
You must be signed in to change notification settings - Fork 345
Allow gitlab URL link shortening from non-gitlab/github.com domains #2068
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
base: main
Are you sure you want to change the base?
Changes from 10 commits
e786e0d
0e43585
9f9df68
617b977
277730f
86dd418
96a1812
2e03182
82339e3
8158d88
0924968
13535e3
394bf57
5a7cda5
6d58054
a5d7086
b55a80e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,8 @@ | |
|
||
class ShortenLinkTransform(SphinxPostTransform): | ||
""" | ||
Shorten link when they are coming from github or gitlab and add an extra class to | ||
the tag for further styling. | ||
Shorten link when they are coming from github, gitlab, or bitbucket and add | ||
an extra class to the tag for further styling. | ||
|
||
Before: | ||
.. code-block:: html | ||
|
@@ -37,9 +37,15 @@ class ShortenLinkTransform(SphinxPostTransform): | |
supported_platform: ClassVar[dict[str, str]] = { | ||
"github.com": "github", | ||
"gitlab.com": "gitlab", | ||
"bitbucket.org": "bitbucket", | ||
} | ||
platform = None | ||
|
||
@classmethod | ||
def add_platform_mapping(cls, platform, netloc): | ||
"""Add domain->platform mapping to class at run-time.""" | ||
cls.supported_platform.update(dict([(netloc, platform)])) | ||
|
||
def run(self, **kwargs): | ||
"""Run the Transform object.""" | ||
matcher = NodeMatcher(nodes.reference) | ||
|
@@ -96,6 +102,22 @@ def parse_url(self, uri: ParseResult) -> str: | |
if parts[2] in ["issues", "pull", "discussions"]: | ||
text += f"#{parts[-1]}" # element number | ||
|
||
elif self.platform == "bitbucket": | ||
# split the url content | ||
parts = path.split("/") | ||
|
||
if len(parts) > 0: | ||
text = parts[0] # organisation | ||
if len(parts) > 1 and not ( | ||
parts[-2] == "workspace" and parts[-1] == "overview" | ||
): | ||
if len(parts) > 1: | ||
text += f"/{parts[1]}" # repository | ||
if len(parts) > 2: | ||
if parts[2] in ["issues", "pull-requests"]: | ||
itemnumber = parts[-1] | ||
text += f"#{itemnumber}" # element number | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is hard to follow, and feels brittle to me. Why aren't we using regex for this? I worked up this gist as a POC and it seems to capture the behavior that we want: https://gist.github.com/drammock/78d2d3c9837aafd1259866c7b936b9e4 presumably similar things will work for other forges (though IIRC gitlab is a bit more complex). @gabalafou WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. The code in this PR follows patterns already established for GitHub and GitLab, but I think those patterns are bad. The way I think this should work is that we should be rather picky about which URL patterns we recognize/support and only shorten those. All the other URLs should be not be shortened. But right now, the code takes the opposite approach, as soon as it sees github.com or gitlab.com, it tries to shorten the URL. For example, let's say we were just starting to support GitHub URLs. But in this scenario, let's start only with supporting pull request URLs. Then we would convert the following link, like so:
But we would not convert any of the following links (if we were only supporting pull request links):
None of those links would get shortened until we specifically add each type of URL that we want to support. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we want to go down the route of a rewrite of the link shortening logic, I would be happy to do it. In that case, I would just close this PR and open a new one. I would abandon the feature in this PR to turn off link shortening because I suspect that part of the motivation for adding a config value to turn it off is because our current link shortener is bad. Perhaps with a better shortener, there would be little to no demand for a config setting to turn it off. What do you think, @mattpitkin? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Since I think I've basically solved it for bitbucket in the linked Gist, I feel like it might be the right call to just rewrite the others too. +1 to close and open a new PR to refactor what we have and also add bitbucket. See also #2215 which adds link shortening for codeberg/forgejo and gitea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do you mean, why did I put the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mattpitkin, sorry, my comment actually didn't make any sense. I was writing it in a hurry yesterday and I got two different PRs mixed up in my head: this one versus #2109. But the other PR is very much related to this PR because it provides an option to turn off link shortening. |
||
|
||
elif self.platform == "gitlab": | ||
# cp. https://docs.gitlab.com/ee/user/markdown.html#gitlab-specific-references | ||
if "/-/" in path and any( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
"""Test conf file.""" | ||
|
||
# -- Project information ----------------------------------------------------- | ||
|
||
project = "Test Self Hosted Version Control URLs" | ||
copyright = "2020, Pydata community" | ||
author = "Pydata community" | ||
|
||
root_doc = "index" | ||
|
||
# -- General configuration --------------------------------------------------- | ||
|
||
# Add any Sphinx extension module names here, as strings. They can be | ||
# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom | ||
# ones. | ||
extensions = [] | ||
html_theme = "pydata_sphinx_theme" | ||
html_context = { | ||
"github_url": "https://github.pydata.org", | ||
"gitlab_url": "https://gitlab.pydata.org", | ||
"bitbucket_url": "https://bitbucket.pydata.org", | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
Test conversion of a self-hosted GitHub URL | ||
=========================================== | ||
|
||
This test ensures that a site using PyData Sphinx Theme can set a self-hosted | ||
version control URL via the theme options and then when the site is built, that | ||
the URLs that go to that self-hosted version control domain will be properly | ||
shortened (just like for github.com, gitlab.com, and bitbucket.org). | ||
|
||
.. toctree:: | ||
:caption: My caption | ||
:numbered: | ||
|
||
links |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
Test Self Hosted Version Control URLs | ||
===================================== | ||
|
||
**normal link** | ||
|
||
- https://pydata-sphinx-theme.readthedocs.io/en/latest/ | ||
|
||
**GitHub** | ||
|
||
.. container:: github-container | ||
|
||
https://github.pydata.org | ||
https://github.pydata.org/pydata | ||
https://github.pydata.org/pydata/pydata-sphinx-theme | ||
https://github.pydata.org/pydata/pydata-sphinx-theme/pull/1012 | ||
https://github.pydata.org/orgs/pydata/projects/2 | ||
|
||
**GitLab** | ||
|
||
.. container:: gitlab-container | ||
|
||
https://gitlab.pydata.org | ||
https://gitlab.pydata.org/gitlab-org | ||
https://gitlab.pydata.org/gitlab-org/gitlab | ||
https://gitlab.pydata.org/gitlab-org/gitlab/-/issues/375583 | ||
https://gitlab.pydata.org/gitlab-org/gitlab/issues/375583 | ||
https://gitlab.pydata.org/gitlab-org/gitlab/-/issues/ | ||
https://gitlab.pydata.org/gitlab-org/gitlab/issues/ | ||
https://gitlab.pydata.org/gitlab-org/gitlab/-/issues | ||
https://gitlab.pydata.org/gitlab-org/gitlab/issues | ||
https://gitlab.pydata.org/gitlab-org/gitlab/-/merge_requests/84669 | ||
https://gitlab.pydata.org/gitlab-org/gitlab/-/pipelines/511894707 | ||
https://gitlab.pydata.org/gitlab-com/gl-infra/production/-/issues/6788 | ||
|
||
**Bitbucket** | ||
|
||
.. container:: bitbucket-container | ||
|
||
https://bitbucket.pydata.org | ||
https://bitbucket.pydata.org/atlassian/workspace/overview | ||
https://bitbucket.pydata.org/atlassian/aui | ||
https://bitbucket.pydata.org/atlassian/aui/pull-requests/4758 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<div class="bitbucket-container docutils container"> | ||
<p> | ||
<a class="bitbucket reference external" href="https://bitbucket.org"> | ||
bitbucket | ||
</a> | ||
<a class="bitbucket reference external" href="https://bitbucket.org/atlassian/workspace/overview"> | ||
atlassian | ||
</a> | ||
<a class="bitbucket reference external" href="https://bitbucket.org/atlassian/aui"> | ||
atlassian/aui | ||
</a> | ||
<a class="bitbucket reference external" href="https://bitbucket.org/atlassian/aui/pull-requests/4758"> | ||
atlassian/aui#4758 | ||
</a> | ||
</p> | ||
</div> |
Uh oh!
There was an error while loading. Please reload this page.