Support non-github repositories

akhmerov commented 4 years ago

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.

choldgraf commented 4 years ago

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

akhmerov commented 4 years ago

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:

choldgraf commented 4 years ago

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

akhmerov commented 4 years ago

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).

choldgraf commented 4 years ago

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

fm75 commented 4 years ago

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.

akhmerov commented 4 years ago

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

fm75 commented 4 years ago

@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 commented 4 years ago

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.

choldgraf commented 4 years ago

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

fm75 commented 4 years ago

@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.

choldgraf commented 4 years ago

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

fm75 commented 4 years ago

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.

choldgraf commented 4 years ago

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

fm75 commented 4 years ago

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

akhmerov commented 3 years ago

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 commented 3 years ago

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.

choldgraf commented 3 years ago

@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)

akhmerov commented 3 years ago

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?

mzme commented 2 years ago

@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?

akhmerov commented 2 years ago

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

leodrivera commented 2 years ago

@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?

tschm commented 2 years ago

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.

executablebooks / sphinx-book-theme