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

Broken 'Edit on GitHub' link #1321

Closed
brianhlin opened this issue Oct 23, 2017 · 8 comments
Closed

Broken 'Edit on GitHub' link #1321

brianhlin opened this issue Oct 23, 2017 · 8 comments
Labels
Bug

Comments

@brianhlin
Copy link

@brianhlin brianhlin commented Oct 23, 2017

We recently updated to 0.17.0 and it seems to have broken our 'Edit on GitHub' link, you can see this in action here. The link given is https://github.com/edit/master/docs/index.md when it should be https://github.com/opensciencegrid/docs/edit/master/docs/index.md. repo_url appears to be set properly.

@waylan
Copy link
Member

@waylan waylan commented Oct 23, 2017

Thanks for the report. It appears that the problem presents itself if you are missing a closing slash at the end of the repo_url. Add the closing slash and it should work correctly:

repo_url: https://github.com/opensciencegrid/docs/

I'm just guessing, but it could be that the added support for fragment and query string based schemes was confused by the lack of a closing slash and messed things up. In any event, it appears that we only tested with the slash, not without. After all, MkDocs own config includes the slash and the example in the documentation shows a slash.

I'm undecided if the closing slash should be a requirement. Although, apparently, it previously worked fine without it, so requiring it would be a regression for some users.

@brianhlin
Copy link
Author

@brianhlin brianhlin commented Oct 23, 2017

Does that mean the site_url should also have a trailing slash?

brianhlin added a commit to brianhlin/technology that referenced this issue Oct 23, 2017
@waylan
Copy link
Member

@waylan waylan commented Oct 23, 2017

This is unique to repo_url. Consider the following examples:

http://example.com/user/project?branch=master&action=edit&path=path/to/page.md
http://example.com/user/project#branch/path/to/page.md
http://example.com/user/project/edit/branch/path/to/page.md

In the first two examples, the repo_url does not end in a slash, but it does in the third example. We support all three. So if your host uses a scheme which requires a slash between the repo_url and the edit_uri, then the setting should include one. However, if your host does not use a slash between the repo_url and the edit_uri, then your setting should not include one. At least that's how it currently works. The question is if that's how it should work.

brianhlin added a commit to brianhlin/technology that referenced this issue Oct 24, 2017
brianhlin added a commit to opensciencegrid/management that referenced this issue Oct 25, 2017
brianhlin added a commit to opensciencegrid/security that referenced this issue Oct 25, 2017
brianhlin added a commit to opensciencegrid/networking that referenced this issue Oct 25, 2017
brianhlin added a commit to opensciencegrid/production that referenced this issue Oct 25, 2017
brianhlin added a commit to opensciencegrid/council that referenced this issue Oct 25, 2017
@waylan
Copy link
Member

@waylan waylan commented Oct 27, 2017

I looked at the tests and realized that we were missing a test. We have a test were repo_url is set to http://example.com but not http://example.com/foo. The tests pass for the first one, but when I added the second, it failed. This is definitely a bug.

We were also missing a test for http://example.com/foo/ (closing slash included) but that works as it should. For now, the workaround is to end the repo_url with a slash.

@waylan waylan added Bug and removed Needs design decision labels Oct 27, 2017
waylan added a commit to waylan/mkdocs that referenced this issue Oct 27, 2017
Also broke up `edit_uri` tests into individual methods and added a few
missing senarious. Fixes mkdocs#1321.
@waylan waylan closed this in #1330 Oct 27, 2017
waylan added a commit that referenced this issue Oct 27, 2017
Also broke up `edit_uri` tests into individual methods and added a few
missing senarious. Fixes #1321.
@dbogatov
Copy link

@dbogatov dbogatov commented Dec 6, 2017

@waylan

My apologies for commenting on a closed issue.

In my setup, mkdocs lives in a subdirectory of the repo.
https://github.com/dbogatov/status-site/tree/master/articles

This setup break edit links because it assumes the docs directory is in the root while it is not.
What would be the best workaround in my case?

@waylan
Copy link
Member

@waylan waylan commented Dec 6, 2017

Then you will need to manually set the edit_uri to properly point to the location of your docs_dir. The "default" only works when you use a default setup. You are not using a default setup, so you need to configure for your setup.

@dbogatov
Copy link

@dbogatov dbogatov commented Dec 7, 2017

@waylan

Wonderful!
Could you post a link to the documentation where this property is explained?
I could not find myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants