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

theme broken for gitlab links with more than subgroup #1039

Closed
nicoa opened this issue Oct 26, 2022 · 5 comments · Fixed by #1048
Closed

theme broken for gitlab links with more than subgroup #1039

nicoa opened this issue Oct 26, 2022 · 5 comments · Fixed by #1048

Comments

@nicoa
Copy link
Contributor

nicoa commented Oct 26, 2022

In #841 link shortening was introduced which is breaking a gitlab (and github) link into several pieces.

This being introduced has two major drawbacks:

  1. it shortens links containing several subgroups wrongly in the sense that it doesn't include the ID and doesn't have the full path until the repository... i.e. https://gitlab.com/organisation/subgroup1/soubgroup2/repository/-/merge_requests/4 will be displayed as organisation/subgroup1 instead of organisation/…/4 or organisation/subgroup1/soubgroup2/repository/4.
  2. it shortens other links (e.g. pipelines) as well, such that https://gitlab.com/organisation/repository/-/pipelines becomes organisation/repository.

To my understanding the feature should only apply to those fields mentioned in https://github.com/pydata/pydata-sphinx-theme/blob/main/src/pydata_sphinx_theme/__init__.py#L958 and should take subgroups into account.
If possible, a configuration entry to opt out would be helpful as a short term solution here as well.

Would you be welcoming a PR in this direction or am I misunderstanding something or don't see another ticket discussing this?

@12rambau
Copy link
Collaborator

We are very welcoming PR and your suggestions seems very sensible. I did coded the shortening function but I have very little knowledge about how gitlab build the url. If you want to edit it it's done here:

class ShortenLinkTransform(SphinxPostTransform):

@nicoa
Copy link
Contributor Author

nicoa commented Nov 4, 2022

Sounds good, Thanks for your feedback! I opened a PR, please leave your suggestions and comments there :)

nicoa added a commit to nicoa/pydata-sphinx-theme that referenced this issue Nov 7, 2022
@nicoa
Copy link
Contributor Author

nicoa commented Nov 9, 2022

Thanks for merging! Does it make sense to add this to the 0.12 or 0.11.1 release?

@12rambau
Copy link
Collaborator

12rambau commented Nov 9, 2022

anything merged to master will be included in the next release

@nicoa
Copy link
Contributor Author

nicoa commented Nov 9, 2022

Ah got it, thx for clarifying! :)

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 a pull request may close this issue.

2 participants