sphinx-book-theme icon indicating copy to clipboard operation
sphinx-book-theme copied to clipboard

Support non-github repositories

Open akhmerov opened this issue 5 years ago • 23 comments

Description

We currently only support GitHub repositories for "repository link" as well as "edit this page". We should support a few of the more common ones as well, such as bitbucket and gitlab.

Here are major repository-specific features we'd want to implement:

  • Repository link / issues link
  • Edit this page link
  • Launch buttons link

Benefit

This would make these theme features usable for people that use these other (quite popular) platforms!

Implementation details

Repository link

For the repository link, here's our code (and likely where a fix would be):

https://github.com/executablebooks/sphinx-book-theme/blob/master/sphinx_book_theme/topbar/repobuttons.html

Edit this page link

We seem to be using the PyData Sphinx Theme get_edit_url function for this already, so we should figure out how to make it support non-GitHub repositories:

https://github.com/executablebooks/sphinx-book-theme/blob/39aaaa3e4d908fe7be8b7378ad474d8c6b86aa16/sphinx_book_theme/topbar/repobuttons.html#L15-L18

here's the PyData theme function for this:

https://github.com/pydata/pydata-sphinx-theme/blob/055ae27cdc6801d6abba0c43bfa9cfd3a359e1b9/pydata_sphinx_theme/init.py#L481

launch buttons

Here's the templating for our launch buttons:

https://github.com/executablebooks/sphinx-book-theme/blob/master/sphinx_book_theme/topbar/launchbuttons.html

And the Python logic that handles how we build these buttons:

https://github.com/executablebooks/sphinx-book-theme/blob/master/sphinx_book_theme/launch.py

Maybe we can re-use the get_edit_url for this somehow as well.

This would require changes here and here.

In particular raw git remotes should cover a lot of use cases.

akhmerov avatar May 31 '20 00:05 akhmerov

For sure - here's where it's hard-coded: https://github.com/executablebooks/sphinx-book-theme/blob/master/sphinx_book_theme/launch.py#L148

we should add the ability to specify different providers and build the right URL depending on what's given

choldgraf avatar Jun 01 '20 03:06 choldgraf

Would it be acceptable to make the whole binder URL a config parameter? Binder now has 8 different sources, all with URLs that shouldn't change often:

image

akhmerov avatar Jun 01 '20 23:06 akhmerov

How do you mean "the whole binder URL"? You mean just the repository and it then figures out the path after that? I guess that could be done as an over-ride for the logic that builds it up by hand...maybe that'd be simpler

choldgraf avatar Jun 05 '20 16:06 choldgraf

I mean having the user provide the complete https://mybinder.org/v2/git/https%3A%2F%2Fgitlab.kwant-project.org%2Fsolidstate%2Flectures.git/master instead of computing the same URL for the user from https://mybinder.org and other inputs (as is done right now).

akhmerov avatar Jun 05 '20 16:06 akhmerov

yeah that's what I was imagining too. I kind-of like that idea rather than manually building it up from the github repo parts, that would simplify things quite a bit and means that we wouldn't have to update things if Binder changes or starts accepting different kinds of URLs

choldgraf avatar Jun 05 '20 17:06 choldgraf

Could I recommend changing the hard-coded function to:

def _split_repo_url(url):
    """Split a repository URL into an org / repo combination."""
    try:
        org, repo = url.split('/')[-2:]
    except:
        SPHINX_LOGGER.warning(
            f"Could not identify org and repo from url {url}"

        )
        org = repo = None
    return org, repo

I will grant that it makes an assumption that we have a well-formed URL that can be split into an org and repo pair, but I would contend that the existing implementation does the same while unnecessarily restricting the domain component of the url.

fm75 avatar Sep 01 '20 15:09 fm75

@fm75 why is splitting into org and repo necessary? Also: it wouldn't work for gitlab which allows for /group/subgroup/subsubgroup/project.

akhmerov avatar Sep 01 '20 15:09 akhmerov

@akhmerov I wanted to leave the API unchanged while freeing it up from hard-coded github.com. The existing code makes it fail for GitHub Enterprise (GHE) use. This would at least prevent spewing the warning. I don't know yet whether it fixes the launch icon issue. I spent all morning looking in jupyter-book trying to figure how to fix these two problems:

  • The Launch icon for .ipynb files only displays for mybinder.com and not some other url.
  • The "Suggest edit" dropdown from the :octocat: links directly to github instead of the actual URL of the source.

I am not familiar with other source control systems. I don't know why the 'org' and 'repo' are necessary, now that you mention it. The Binder URL generator does not need them in any way that I can see. I also have not yet what exactly gets input into Sphinx.

fm75 avatar Sep 01 '20 15:09 fm75

I forked this and tested the code I suggested above.

  • The Launcher now shows up on a notebook page (good)
    • The binder URL now is the same as the binderhub_url value in launch_buttons in _config.yml (good)
    • But it misses because it has "//v2/gh/..." instead of "/v2/gh/..." (not good yet)
    • I need to know where the rest of the binder URL is built. Except for the // it would be working.
  • It does not solve the :octocat: "suggest edit" link which is hard-wired to github.com, even though the repository and open issue URLs are correct
    • I have not yet found that code, either.

fm75 avatar Sep 01 '20 21:09 fm75

note that this is where the link is generated for the "edit this page" button:

https://github.com/executablebooks/sphinx-book-theme/blob/master/sphinx_book_theme/topbar/repobuttons.html

it is actually using a function to return the edit URL that is defined in the pydata sphinx theme:

https://github.com/pandas-dev/pydata-sphinx-theme/blob/master/pydata_sphinx_theme/init.py#L142

so we could start off by using our own function that behaves as we'd expect, and can plan to upstream this later on.

I think the edits to accommodate non-github instance would need to be there + maybe some logic here to make it easier to build the URLs:

https://github.com/executablebooks/sphinx-book-theme/blob/master/sphinx_book_theme/init.py#L196

choldgraf avatar Sep 01 '20 23:09 choldgraf

@choldgraf Thanks. Except for figuring out the branch and whether the doc is .md or .ipynb, I can just modify repobuttons.html and don't need the get_edit_url function, at all. Are there "fields" available to repobuttons.html that have branch information and the file extension?

That said, I am not yet clear enough on what the overall architecture is here, so I don't know where I would implement a replacement function. From looking at the pydata-sphinx-theme, it seems like this is piggy-backed onto a project outside the executable-books "universe". It also appears that that theme is strongly coupled to github or a github-like architecture.

It seems that much more would be needed to implement a get_edit_url to support source repositories that do not have the same structure as github/GHE.

fm75 avatar Sep 02 '20 13:09 fm75

I'd just mimic how the "issues button" is created in the sphinx-book-theme template, and follow a similar pattern for the "edit" button instead of using the get_edit_url function from pydata. I think longer-term to support more repository websites, we probably do want a function to generate a variety of links depending on github/gitlab/bitbucket/etc

choldgraf avatar Sep 04 '20 20:09 choldgraf

I'd just mimic how the "issues button" is created in the sphinx-book-theme template, and follow a similar pattern for the "edit" button instead of using the get_edit_url function from pydata. I think longer-term to support more repository websites, we probably do want a function to generate a variety of links depending on github/gitlab/bitbucket/etc

Thanks for the suggestion. I have not figured out how to tell the type of document. My current (ugly) hack is to create two buttons in the template, one each for .md and .ipynb. One will work and the other won't.

fm75 avatar Sep 09 '20 16:09 fm75

could you try the pattern used here: https://github.com/executablebooks/sphinx-book-theme/blob/master/sphinx_book_theme/topbar/launchbuttons.html#L21 ?

choldgraf avatar Sep 09 '20 17:09 choldgraf

Thanks. That worked. Just did not know about page_source_suffix.

fm75 avatar Sep 10 '20 14:09 fm75

I've inspected get_edit_url from pydata theme, and it seems that one actually supports gitlab, bitbucket, or github enterprise. However book theme reduces support of alternatives by setting the context on its own over here:

https://github.com/executablebooks/sphinx-book-theme/blob/2a895acdbe0bc958a5842f1c9e1fe58afe8026b0/sphinx_book_theme/init.py#L170-L177

That's seems unfortunate: there are now two ways to provide the information and one overrides the other with no good reason.

akhmerov avatar Sep 26 '21 11:09 akhmerov

For those coming to this issue, here's a workaround in conf.py that I used for repository buttons:

html_context = {
    "gitlab_url": "...",
    "gitlab_user": "...",
    "gitlab_repo": "...",
    "gitlab_version": "master",
    "doc_path": "source",
}


def cleanup_context(app, pagename, templatename, context, doctree):
    """Override sphinx-book-theme github-centric approach.

    See https://github.com/executablebooks/sphinx-book-theme/issues/114
    """
    for key in ["github_user", "github_repo", "github_version", "theme_github_url"]:
        context.pop(key, None)


def setup(app):
    app.connect("html-page-context", cleanup_context, priority=999)

I've also replaced fa-github with fa-gitlab in _templates/topbar/repobuttons.html for prettiness.

akhmerov avatar Sep 26 '21 12:09 akhmerov

@akhmerov I suspect that the difference in implementation is because the pydata theme didn't use to be able to do this. I am definitely +1 on deprecating our implementation here in favor of re-using the pydata theme config + template function:

https://github.com/pydata/pydata-sphinx-theme/blob/055ae27cdc6801d6abba0c43bfa9cfd3a359e1b9/pydata_sphinx_theme/init.py#L481

Is that something you'd be interested in trying out? Is there any blocker you can think of for why we can't just re-use the pydata theme function?

(I tried updating the top comment with some more fleshed-out information)

choldgraf avatar Sep 26 '21 15:09 choldgraf

Thanks for the explanation. I am happy to give it a shot. My main question, however, is how to deal with backwards compatibility and the deprecations. Do you have any suggestions or preferences?

akhmerov avatar Sep 27 '21 12:09 akhmerov

@akhmerov
Thanks for the workaround in the conf.py. It worked well for 0.2.0. In 0.3.2 it is not working anymore. Do you have a suggestion to adapt the workaround?

mzme avatar Mar 31 '22 14:03 mzme

Sorry, not off the top of my head; this would need a code dive.

akhmerov avatar Apr 05 '22 08:04 akhmerov

@choldgraf and @akhmerov

I'm currently having the same need to use non-github repositories and by seeing your discussion I also agree that it would be a good idea to use pydata theme config + template function. Did you check any restrictions to not implement this way?

leodrivera avatar Jul 21 '22 22:07 leodrivera

I am currently experimenting with Gitlab hosted by us. The functionality to suggest an edit could be interesting but it seems the hardcoded GitHub connection has survived.

tschm avatar Aug 05 '22 19:08 tschm