Repository: mkdocs/mkdocs

Issue:
'Edit on Github' should link to the page, not the project
IMO, the Github link should be smartened so that its text is 'Edit on Github' and the hyperlink points to the current page. For example, see the link in upper right of http://read-the-docs.readthedocs.org/en/latest/builds.html

PIP docs [do this well](http://pip.readthedocs.org/en/latest/news.html)

Discussion:
lorengordon: I had a need for this feature and didn't see further progress on it since the earlier PRs were closed, so I took a stab at it. I did study https://github.com/mkdocs/mkdocs/pull/752 and https://github.com/mkdocs/mkdocs/pull/802, and read through the original issue https://github.com/mkdocs/mkdocs/issues/269. Hopefully I addressed the concerns and this aligns with the desired approach. Let me know if you'd like me to adjust anything.
waylan: @lorengordon overall this looks pretty good. I made a few inline comments. Also, I don't see any changes to the `mkdocs` theme. Make those changes and I think this will be good.
waylan: Oh, I almost forgot, the new template context  variable needs to be documented [here][1]. Just add an entry for `page.edit_url`.

[1]: https://github.com/mkdocs/mkdocs/blob/master/docs/user-guide/custom-themes.md#page
lorengordon: @waylan, I'm on it. This is my first time contributing to mkdocs... Any preference for amending the prior commit vs adding a second? I didn't see any particular guidance in the contributing doc.
waylan: We prefer a clean commit history, so if you can amend the previous commit and do a `--force` push, that would be preferred,  but if not we can work with that as well.
lorengordon: :+1: Yep, my preference as well. It's not everyone's, so I like to check first.
lorengordon: @waylan, ok, that should address the comments. Anything else?
lorengordon: @waylan, further comments addressed.
lorengordon: Alright, think the last line comment is now covered.
waylan: Thanks. This looks good to me.

The only concern is that this does add a new config setting. I think its necessary to support the feature, but I would like an okay from @d0ugal before merging it. If he's not okay with that, then we would have to hardcode the `edit_uri` value in for each supported `repo`. Of course, the current patch gives us support for most any hosting service out-of-the-box, whereas hardcoding the `edit_uri` would limit support only to the hosting services we code for.

Although, it occurs to me that we could hardcode some defaults for the new setting (if `repo_name` is `github`, default to this or if `repo_name` is `bitbucket` default to that). That way, users would only need to set the value if they want to use a non-standard URL (a different branch than master) on a known service, or an unknown (to us) service (such as GitLab).

For completeness, I should also note that now that we have #947, its possible for users to easily override the template and put anything they want in the "edit this page link" via the `repo` block. While template customization is certainly more difficult for users than altering a setting, it  doesn't require us to offer support for any specific feature and gives the users much more power to do whatever they want. I would think this is the strongest argument against adding a new setting for this feature.

However, as we specifically advertise this project as a tool for documenting your coding projects, explicitly offering this feature with a config setting is a reasonable expectation for users to have.
lorengordon: Yes, one goal of mine was to support arbitrary repo hosts and branches. I personally like to use a `develop` branch, and would prefer to send users there rather than whatever branch the `repo` host likes to "default" to.

I did take note of the comment on https://github.com/mkdocs/mkdocs/pull/752 about minimizing the number of config options, which is why I combined everything into one setting. Plus, I think it is more flexible this way, anyway.

It appears the config_option classes are able to lookup the config dictionary, so it looks trivial to add a new class, `EditUri()`, similar to the approach for `RepoURL()`, that sets defaults for `GitHub` and `Bitbucket`.

Or, hmm, don't want to run into ordering issues with the config file. Perhaps just extending `RepoURL()` would work? I'll twizzle something out...
lorengordon: Also, `urljoin()` doesn't appear to work that way. Without the trailing slash on the `base` param, the last fragment gets dropped. So this:

`urljoin('https://example.com/foo', 'bar.md')`

returns this:

`'https://example.com/bar.md'`.

https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urljoin
lorengordon: Anyway, I added [this bit](https://github.com/mkdocs/mkdocs/pull/975/files#diff-21a6a972ad3c1097b825235b2e9b4848R205) to set default values for GitHub and Bitbucket. Does that seem reasonable?
lorengordon: Going that approach, would it be valuable to add a few unit tests to the [RepoURLTest](https://github.com/mkdocs/mkdocs/blob/master/mkdocs/tests/config/config_options_tests.py#L102-L126) class?

```
    def test_edit_uri_github(self):

        option = config_options.RepoURL()
        config = {'repo_url': "https://github.com/mkdocs/mkdocs"}
        option.post_validation(config, 'repo_url')
        self.assertEqual(config['edit_uri'], 'blob/master/docs/')

    def test_edit_uri_bitbucket(self):

        option = config_options.RepoURL()
        config = {'repo_url': "https://bitbucket.org/gutworth/six/"}
        option.post_validation(config, 'repo_url')
        self.assertEqual(config['edit_uri'], 'src/default/docs/')

    def test_edit_uri_custom(self):

        option = config_options.RepoURL()
        config = {'repo_url': "https://launchpad.net/python-tuskarclient"}
        option.post_validation(config, 'repo_url')
        self.assertEqual(config.get('edit_uri'), None)
```
waylan: Yes, some more tests would be a good idea. Actually, we should probably add a few tests of the new `Page.set_edit_url` method as well. Just create a few dummy `Page` objects and confirm the method returns the correct result with various input.
lorengordon: Can you give me a few more pointers for the `Page` tests? I looked at [nav_tests.py](https://github.com/mkdocs/mkdocs/blob/master/mkdocs/tests/nav_tests.py) and poked around a few others, but it wasn't clear how that part of the code was being tested.

Patch:
diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md
index 422e7c90..6fbebe23 100644
--- a/docs/user-guide/configuration.md
+++ b/docs/user-guide/configuration.md
@@ -50,6 +50,37 @@ When set, provides a link to your GitHub or Bitbucket repository on each page.
 **default**: `'GitHub'` or `'Bitbucket'` if the `repo_url` matches those
 domains, otherwise `null`
 
+### edit_uri
+
+Path from the base `repo_url` to the docs directory when directly viewing a
+page, accounting for specifics of the repository host (e.g. GitHub, Bitbucket,
+etc), the branch, and the docs directory itself. Mkdocs concatenates `repo_url`
+and `edit_uri`, and appends the input path of the page.
+
+When set, provides a link directly to the page in your source repository. This
+makes it easier to find and edit the source for the page. If `repo_url` is not
+set, this option is ignored.
+
+For example, for a GitHub-hosted repository, the `edit_uri` would be as follows.
+(Note the `blob` path and `master` branch...)
+
+```yaml
+edit_uri: blob/master/docs/
+```
+
+For a Bitbucket-hosted repository, the equivalent `edit_uri` would be as
+follows. (Note the `src` path and `default` branch...)
+
+```yaml
+edit_uri: src/default/docs/
+```
+
+For other repository hosts, `edit_uri` works the same way. Simply specify the
+relative path to the docs directory.
+
+**default**: `blob/master/docs/` or `src/default/docs/` for GitHub or Bitbucket
+repos, respectively, if `repo_url` matches those domains, otherwise `null`
+
 ### site_description
 
 Set the site description. This will add a meta tag to the generated HTML header.
diff --git a/docs/user-guide/custom-themes.md b/docs/user-guide/custom-themes.md
index cd4d8e8a..10df837a 100644
--- a/docs/user-guide/custom-themes.md
+++ b/docs/user-guide/custom-themes.md
@@ -259,6 +259,11 @@ documentation page.
 The full, canonical URL to the current page. This includes the `site_url` from
 the configuration.
 
+##### page.edit_url
+
+The full URL to the input page in the source repository. Typically used to
+provide a link to edit the source page.
+
 ##### page.url
 
 The URL to the current page not including the `site_url` from the configuration.
diff --git a/mkdocs/commands/build.py b/mkdocs/commands/build.py
index a4ce837c..83e1a1c6 100644
--- a/mkdocs/commands/build.py
+++ b/mkdocs/commands/build.py
@@ -116,6 +116,9 @@ def get_page_context(page, content, toc, meta, config):
     if config['site_url']:
         page.set_canonical_url(config['site_url'])
 
+    if config['repo_url']:
+        page.set_edit_url(config['repo_url'], config['edit_uri'])
+
     page.content = content
     page.toc = toc
     page.meta = meta
diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py
index e9065e00..25dd8b09 100644
--- a/mkdocs/config/config_options.py
+++ b/mkdocs/config/config_options.py
@@ -202,6 +202,12 @@ class RepoURL(URL):
             else:
                 config['repo_name'] = repo_host.split('.')[0].title()
 
+        if config['repo_url'] is not None and config.get('edit_uri') is None:
+            if config['repo_name'].lower() == 'github':
+                config['edit_uri'] = 'blob/master/docs/'
+            elif config['repo_name'].lower() == 'bitbucket':
+                config['edit_uri'] = 'src/default/docs/'
+
 
 class Dir(Type):
     """
diff --git a/mkdocs/config/defaults.py b/mkdocs/config/defaults.py
index 3d8c71e2..a5ff9d77 100644
--- a/mkdocs/config/defaults.py
+++ b/mkdocs/config/defaults.py
@@ -75,6 +75,12 @@ DEFAULT_SCHEMA = (
     # "GitHub" or "Bitbucket" for known url or Hostname for unknown urls.
     ('repo_name', config_options.Type(utils.string_types)),
 
+    # Specify a URI to the docs dir in the project source repo, relative to the
+    # repo_url. When set, a link directly to the page in the source repo will
+    # be added to the generated HTML. If repo_url is not set also, this option
+    # is ignored.
+    ('edit_uri', config_options.Type(utils.string_types)),
+
     # Specify which css or javascript files from the docs directory should be
     # additionally included in the site. Default, List of all .css and .js
     # files in the docs dir.
diff --git a/mkdocs/nav.py b/mkdocs/nav.py
index 4c5b08f2..3b4da601 100644
--- a/mkdocs/nav.py
+++ b/mkdocs/nav.py
@@ -151,6 +151,7 @@ class Page(object):
         # Placeholders to be filled in later in the build
         # process when we have access to the config.
         self.canonical_url = None
+        self.edit_url = None
         self.content = None
         self.meta = None
         self.toc = None
@@ -186,6 +187,18 @@ class Page(object):
             base += '/'
         self.canonical_url = utils.urljoin(base, self.abs_url.lstrip('/'))
 
+    def set_edit_url(self, repo_url, edit_uri):
+        if not repo_url.endswith('/'):
+            repo_url += '/'
+        if not edit_uri:
+            self.edit_url = repo_url
+        else:
+            if not edit_uri.endswith('/'):
+                edit_uri += '/'
+            self.edit_url = utils.urljoin(
+                repo_url + edit_uri,
+                self.input_path)
+
 
 class Header(object):
     def __init__(self, title, children):
diff --git a/mkdocs/themes/mkdocs/nav.html b/mkdocs/themes/mkdocs/nav.html
index bf3a8e77..7def66d6 100644
--- a/mkdocs/themes/mkdocs/nav.html
+++ b/mkdocs/themes/mkdocs/nav.html
@@ -69,9 +69,9 @@
               {%- endblock %}
 
               {%- block repo %}
-                {%- if repo_url %}
+                {%- if page and page.edit_url %}
                     <li>
-                        <a href="{{ repo_url }}">
+                        <a href="{{ page.edit_url }}">
                             {%- if repo_name == 'GitHub' %}
                                 <i class="fa fa-github"></i>
                             {%- elif repo_name == 'Bitbucket' -%}
diff --git a/mkdocs/themes/readthedocs/breadcrumbs.html b/mkdocs/themes/readthedocs/breadcrumbs.html
index e1a4f3a9..c169cd6c 100644
--- a/mkdocs/themes/readthedocs/breadcrumbs.html
+++ b/mkdocs/themes/readthedocs/breadcrumbs.html
@@ -13,12 +13,13 @@
     {% if page %}<li>{{ page.title }}</li>{% endif %}
     <li class="wy-breadcrumbs-aside">
       {%- block repo %}
-      {% if repo_url %}
-        {% if repo_name == 'GitHub' %}
-          <a href="{{ repo_url }}" class="icon icon-github"> Edit on GitHub</a>
-        {% elif repo_name == 'Bitbucket' %}
-          <a href="{{ repo_url }}" class="icon icon-bitbucket"> Edit on BitBucket</a>
-        {% endif %}
+      {% if page and page.edit_url %}
+        <a href="{{ page.edit_url }}"
+        {%- if repo_name|lower == 'github' %}
+          class="icon icon-github"
+        {%- elif repo_name|lower == 'bitbucket' %}
+          class="icon icon-bitbucket"
+        {% endif %}> Edit on {{ repo_name }}</a>
       {% endif %}
       {%- endblock %}
     </li>


Test patch:
diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py
index 3c040102..05898b19 100644
--- a/mkdocs/tests/config/config_options_tests.py
+++ b/mkdocs/tests/config/config_options_tests.py
@@ -125,6 +125,27 @@ class RepoURLTest(unittest.TestCase):
         self.assertEqual(config['repo_url'], config['repo_url'])
         self.assertEqual(config['repo_name'], "Launchpad")
 
+    def test_edit_uri_github(self):
+
+        option = config_options.RepoURL()
+        config = {'repo_url': "https://github.com/mkdocs/mkdocs"}
+        option.post_validation(config, 'repo_url')
+        self.assertEqual(config['edit_uri'], 'blob/master/docs/')
+
+    def test_edit_uri_bitbucket(self):
+
+        option = config_options.RepoURL()
+        config = {'repo_url': "https://bitbucket.org/gutworth/six/"}
+        option.post_validation(config, 'repo_url')
+        self.assertEqual(config['edit_uri'], 'src/default/docs/')
+
+    def test_edit_uri_custom(self):
+
+        option = config_options.RepoURL()
+        config = {'repo_url': "https://launchpad.net/python-tuskarclient"}
+        option.post_validation(config, 'repo_url')
+        self.assertEqual(config.get('edit_uri'), None)
+
 
 class DirTest(unittest.TestCase):