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

Add "Edit on Github" links to the documentation. #347

Merged
merged 8 commits into from
Sep 24, 2012

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented Aug 16, 2012

This is a first pass at adding "Edit on Github" links to the documentation mentioned in #329.

This works in a similar way to the feature in Plone it that it links to the source file in the github tree which then gives the user the opportunity to press the "Edit" button and then submit a pull request.

This is completely new code, however -- the Plone feature doesn't allow for editing docstrings, only the flat RST files.

It seems to be quite functional, but it's ugly as all get out :) Perhaps someone with more HTML/CSS fu can help with that aspect.

@embray
Copy link
Member

embray commented Aug 16, 2012

What's ugly about the HTML? Looks like it's generated programatically. Or will I just know when I see it? :)

@mdboom
Copy link
Contributor Author

mdboom commented Aug 17, 2012

The link just looks cumbersome when rendered. As code it's perfectly fine, I suppose.

@embray
Copy link
Member

embray commented Aug 17, 2012

I'll have a go at improving it in a bit.

@embray
Copy link
Member

embray commented Aug 17, 2012

I've got a PR in progress on touching up some of the display. But one other bug I noticed is that there needs to be a way to exclude the "edit this page" link from certain pages, specifically anything under _generated :)

@mdboom
Copy link
Contributor Author

mdboom commented Aug 17, 2012

Thanks for noticing the "_generated" thing. I'll address that. I suspect that excluding any path beginning with an underscore should probably be sufficient.

@mdboom
Copy link
Contributor Author

mdboom commented Aug 17, 2012

A couple of additional things to note:

This is set up to reference the branch on github by githash. This means the entire document tree needs to be rebuilt every time the githash changes (unless there's some clever sphinx-fu to insert the branch later in the document-building process). This also means that if you build the docs for a local revision that hasn't yet been pushed to the main repo, the links will fail. An alternative would be to reference the version tag or "master" if the astropy version has "dev" in it. That's imperfect also, because there could be a version mismatch such that the line numbers no longer line up. None of this is a deal breaker for the intended use case of online docs of each released version, but thought I should mention it.

@embray
Copy link
Member

embray commented Aug 17, 2012

I noticed that too, but decided it wasn't a big deal. If you select a branch manually it still takes you to the correct line in the correct file (provided that it exists in that branch).

@astrofrog
Copy link
Member

This is great! However, when I click on 'edit this page' and I get taken to github, it says I have to be on a branch to be able to make any changes. This is from a local build of your branch, without modifications. Is this because it is not yet merged into the core?

@mdboom
Copy link
Contributor Author

mdboom commented Aug 18, 2012

@astrofrog: Indeed this is a problem. It was using the git hash as a "branch", which, while it points to the correct place in the correct file, doesn't work for editing. I've changed this logic so that:

a) If the version has "dev" in it, it goes to "master"
b) Otherwise, it goes to the corresponding version tag.

Online editing seems to work with tags, even though the error message explicitly says "branch".

@astrofrog
Copy link
Member

If I go to the NDData index page and click edit, I get sent to

https://github.com/astropy/astropy/blob/master/docs/nddata/index.rst

Is there a way to get sent to

https://github.com/astropy/astropy/edit/master/docs/nddata/index.rst

instead?

@mdboom
Copy link
Contributor Author

mdboom commented Aug 18, 2012

Sure -- I think I changed it to "blob" when it was using githashes, where "edit" gives a 404. I'll change this to go to the "edit" URL.

@mdboom
Copy link
Contributor Author

mdboom commented Aug 18, 2012

On further experimentation, this isn't the best idea, because it can't jump directly to the correct line number on an edit page (there are no line anchors). Even though it requires the extra step of clicking on the edit button, I think it's somehow more obvious what needs to be edited if it jumps to the right place in the file.

Secondly, the "edit" URL gives a 404 if the user is not signed in. As this feature is mainly intended for non-developers, who may not even have a github account, I think it's better that they get somewhere other than a 404 when clicking on these links. The "Edit" button will then give the help message that the user needs to sign in in order to edit.

FWIW, the Plone approach to this provides a great deal of instruction:

http://opensourcehacker.com/2012/01/08/readthedocs-org-github-edit-backlink-and-short-history-of-plone-documentation/

@embray
Copy link
Member

embray commented Aug 20, 2012

I haven't looked too closely at it, but should we just copy Plone's approach then?

@mdboom
Copy link
Contributor Author

mdboom commented Aug 20, 2012

The Plone code does not allow for editing docstrings, only rst files. Also, those instructions are out-of-date wrt how github currently works, making me think it's probably best to leave the instructions out -- it's pretty obvious what to do in any case, particularly given how github currently is layed out.

@astrofrog
Copy link
Member

@mdboom - is this ready for a final review/testing?

@mdboom
Copy link
Contributor Author

mdboom commented Aug 30, 2012

Yes -- I think this is ready.

@eteq
Copy link
Member

eteq commented Sep 5, 2012

It looks to me like this is only in the astropy core docs/conf.py, not in the master conf.py shared with affiliated packages. Is there a technical reason why this can't be included in the affiliated packages, or just that you need to specify the project name and branch and such?

@eteq
Copy link
Member

eteq commented Sep 6, 2012

Another thing about this: right now the only people that can actually edit are those with commit rights, is that correct? Perhaps the text should be "view on github" or something like that, given that most doc readers can't actually edit directly... Or does it give an option to issue a PR or something if you don't have commit rights?

(But details aside, this is really cool - nice job, @mdboom !)

@mdboom
Copy link
Contributor Author

mdboom commented Sep 7, 2012

Anyone can fork the repository and submit a pull request if they don't have commit rights. Github walks you through the process if that's the case.

@mdboom
Copy link
Contributor Author

mdboom commented Sep 7, 2012

To answer the question about affiliated packages: There is nothing preventing this -- only that they have to define the configuration variables.

@astrofrog
Copy link
Member

I have an idea - we could always have a page in the docs that explicitly states how the edit feature works, and that once they click on 'edit', they have to click on 'edit' again once they are on the source page. Then, next to the 'Edit' link in the docs, we have a 'Help' link? So:

[Edit on Github] [Help]

or something like this?

@eteq
Copy link
Member

eteq commented Sep 8, 2012

@mdboom - Ah, that's actually really cool! In that case, I recind my comment :)

@astrofrog - That might be a good idea... I admit it took me a minute to notice the edit button when I first tested it...

@astrofrog
Copy link
Member

Actually what would be even nicer is if the [Help] button pops up a window like what you get when you click on 'GitHub Flavored Markdown'. I'll try and experiment with this.

@astrofrog
Copy link
Member

Actually, for now, we should just create a help page and we can get fancy with css overlays later...

@jakebiesinger
Copy link

Hi!

I found your solution while trying to get the same functionality on my own repo. I ended up sticking the page-specific edits into the This Page sidebar, replacing "View Source" with a github /blob/ link and the "Edit Page" link with an /edit/ link.

My solution for the [Help] page was just to set the link title, which gives a tooltip hint (e.g., title="Edit on GitHub (404 if not logged in).")

jakebiesinger/ruffus@6a07ce9

You also have to explicitly expose the conf.py variables in the html_context dictionary
https://github.com/jakebiesinger/ruffus/blob/6a07ce9e00a39dc10d20f7b34abdb68b653ca07b/doc/conf.py#L258

@astrofrog
Copy link
Member

I think we should just go ahead and merge this. Better is the enemy of good, and we can focus on adding help/docs for this later. It'd be great to get the functionality in there. @mdboom - feel free to merge!

@mdboom
Copy link
Contributor Author

mdboom commented Sep 24, 2012

@jakebiesinger: This takes a slightly different approach -- all of the links go to the "blob", and then the user must push the "edit" button, which is greyed out (with a helpful tooltip) if they are not logged in, so there's no chance of a 404.

You raise a good point, though, that the "Source" and "Edit on Github" links are now somewhat redundant -- one taking to a local flat file, the other taking to the github blob page. I think we can address that in a subsequent PR if that redundancy seems confusing.

I am going to pursue your idea of putting something the the title attribute before merging this. That seems like a good idea and nice and lightweight.

mdboom added a commit that referenced this pull request Sep 24, 2012
Add "Edit on Github" links to the documentation.
@mdboom mdboom merged commit 3cae259 into astropy:master Sep 24, 2012
@keflavich
Copy link
Contributor

Great extension; I'm making use of it in some other projects now. I also made a (nearly trivial) bitbucket extension, in case anyone is using bitbucket: https://bitbucket.org/pyspeckit/pyspeckit.bitbucket.org/src/tip/doc/sphinxext/edit_on_bitbucket.py (though right now, bitbucket doesn't have github's on-page editor)

keflavich pushed a commit to keflavich/astropy that referenced this pull request Oct 9, 2013
Add "Edit on Github" links to the documentation.
@mdboom mdboom deleted the doc/edit_on_github branch May 21, 2014 23:54
@westurner
Copy link

I packaged and extended a script with GitHub or BitBucket support as https://pypi.python.org/pypi/sphinxcontrib-srclinks . Thanks!

@embray
Copy link
Member

embray commented Jan 12, 2015

@westurner Cool, thanks! Maybe now we could turn around and use your fork instead :)

@westurner
Copy link

Cool. Friendly forks and pull requests are welcome.

On Jan 12, 2015 12:42 PM, "Erik Bray" notifications@github.com wrote:

@westurner Cool, thanks! Maybe now we could turn around and use your fork
instead :)


Reply to this email directly or view it on GitHub.

astrofrog pushed a commit to astropy/sphinx-astropy that referenced this pull request Jan 29, 2018
Add "Edit on Github" links to the documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants