From be9f10d9c638ee5577d0a438176d8dd1dafb5076 Mon Sep 17 00:00:00 2001 From: M Bussonnier Date: Tue, 25 Jun 2024 12:03:36 +0200 Subject: [PATCH] Fix url shortening for gitlab, and more test for github (#1888) --- src/pydata_sphinx_theme/short_link.py | 19 +++-- tests/sites/base/page1.rst | 2 + tests/test_build/gitlab_links.html | 6 ++ tests/test_short_url.py | 108 ++++++++++++++++++++++++++ 4 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 tests/test_short_url.py diff --git a/src/pydata_sphinx_theme/short_link.py b/src/pydata_sphinx_theme/short_link.py index 082d26774..ca5d265c6 100644 --- a/src/pydata_sphinx_theme/short_link.py +++ b/src/pydata_sphinx_theme/short_link.py @@ -63,7 +63,6 @@ def parse_url(self, uri: ParseResult) -> str: the reformated url title """ path = uri.path - if path == "": # plain url passed, return platform only return self.platform @@ -99,12 +98,18 @@ def parse_url(self, uri: ParseResult) -> str: map(uri.path.__contains__, ["issues", "merge_requests"]) ): group_and_subgroups, parts, *_ = path.split("/-/") - parts = parts.split("/") - url_type, element_number, *_ = parts - if url_type == "issues": - text = f"{group_and_subgroups}#{element_number}" - elif url_type == "merge_requests": - text = f"{group_and_subgroups}!{element_number}" + parts = parts.rstrip("/") + if "/" not in parts: + text = f"{group_and_subgroups}/{parts}" + else: + parts = parts.split("/") + url_type, element_number, *_ = parts + if not element_number: + text = group_and_subgroups + elif url_type == "issues": + text = f"{group_and_subgroups}#{element_number}" + elif url_type == "merge_requests": + text = f"{group_and_subgroups}!{element_number}" else: # display the whole uri (after "gitlab.com/") including parameters # for example "///" diff --git a/tests/sites/base/page1.rst b/tests/sites/base/page1.rst index 623ac572e..ce3393abb 100644 --- a/tests/sites/base/page1.rst +++ b/tests/sites/base/page1.rst @@ -24,7 +24,9 @@ Page 1 https://gitlab.com/gitlab-org/gitlab https://gitlab.com/gitlab-org/gitlab/-/issues/375583 https://gitlab.com/gitlab-org/gitlab/issues/375583 + https://gitlab.com/gitlab-org/gitlab/-/issues/ https://gitlab.com/gitlab-org/gitlab/issues/ + https://gitlab.com/gitlab-org/gitlab/-/issues https://gitlab.com/gitlab-org/gitlab/issues https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84669 https://gitlab.com/gitlab-org/gitlab/-/pipelines/511894707 diff --git a/tests/test_build/gitlab_links.html b/tests/test_build/gitlab_links.html index 3c57ca5a7..f12fcba43 100644 --- a/tests/test_build/gitlab_links.html +++ b/tests/test_build/gitlab_links.html @@ -15,9 +15,15 @@ gitlab-org/gitlab/issues/375583 + + gitlab-org/gitlab/issues + gitlab-org/gitlab/issues/ + + gitlab-org/gitlab/issues + gitlab-org/gitlab/issues diff --git a/tests/test_short_url.py b/tests/test_short_url.py new file mode 100644 index 000000000..37af9a85e --- /dev/null +++ b/tests/test_short_url.py @@ -0,0 +1,108 @@ +"""Shortening url tests.""" + +from urllib.parse import urlparse + +import pytest +from pydata_sphinx_theme.short_link import ShortenLinkTransform + + +class Mock: + """mock object.""" + + pass + + +@pytest.mark.parametrize( + "platform,url,expected", + [ + # TODO, I belive this is wrong as both github.com and github.com/github + # shorten to just github. + ("github", "https://github.com", "github"), + ("github", "https://github.com/github", "github"), + ("github", "https://github.com/pydata", "pydata"), + ( + "github", + "https://github.com/pydata/pydata-sphinx-theme", + "pydata/pydata-sphinx-theme", + ), + ( + "github", + "https://github.com/pydata/pydata-sphinx-theme/pull/1012", + "pydata/pydata-sphinx-theme#1012", + ), + # TODO, I belive this is wrong as both orgs/pydata/projects/2 and pydata/projects/issue/2 + # shorten to the same + ("github", "https://github.com/orgs/pydata/projects/2", "pydata/projects#2"), + ("github", "https://github.com/pydata/projects/pull/2", "pydata/projects#2"), + # issues and pulls are athe same, so it's ok to normalise to the same here + ("github", "https://github.com/pydata/projects/issues/2", "pydata/projects#2"), + # Gitlab + ("gitlab", "https://gitlab.com/tezos/tezos/-/issues", "tezos/tezos/issues"), + ("gitlab", "https://gitlab.com/tezos/tezos/issues", "tezos/tezos/issues"), + ( + "gitlab", + "https://gitlab.com/gitlab-org/gitlab/-/issues/375583", + "gitlab-org/gitlab#375583", + ), + ( + # TODO, non canonical url, discuss if should maybe be shortened to + # gitlab-org/gitlab#375583 + "gitlab", + "https://gitlab.com/gitlab-org/gitlab/issues/375583", + "gitlab-org/gitlab/issues/375583", + ), + ( + "gitlab", + "https://gitlab.com/gitlab-org/gitlab/-/issues/", + "gitlab-org/gitlab/issues", + ), + ( + # TODO, non canonical url, discuss if should maybe be shortened to + # gitlab-org/gitlab/issues (no trailing slash) + "gitlab", + "https://gitlab.com/gitlab-org/gitlab/issues/", + "gitlab-org/gitlab/issues/", + ), + ( + "gitlab", + "https://gitlab.com/gitlab-org/gitlab/-/issues", + "gitlab-org/gitlab/issues", + ), + ( + "gitlab", + "https://gitlab.com/gitlab-org/gitlab/issues", + "gitlab-org/gitlab/issues", + ), + ( + "gitlab", + "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84669", + "gitlab-org/gitlab!84669", + ), + ( + "gitlab", + "https://gitlab.com/gitlab-org/gitlab/-/pipelines/511894707", + "gitlab-org/gitlab/-/pipelines/511894707", + ), + ( + "gitlab", + "https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6788", + "gitlab-com/gl-infra/production#6788", + ), + ], +) +def test_shorten(platform, url, expected): + """Unit test for url shortening. + + Usually you also want a build test in `test_build.py` + """ + document = Mock() + document.settings = Mock() + document.settings.language_code = "en" + document.reporter = None + + sl = ShortenLinkTransform(document) + sl.platform = platform + + URI = urlparse(url) + + assert sl.parse_url(URI) == expected