fix: apply review suggestions

__tests__/integration/fixtures/ok-include-path-edit-uri/docs/other.md

@@ -0,0 +1 @@
+# Other Documentation

__tests__/integration/fixtures/ok-include-path-edit-uri/mkdocs.yml

@@ -8,4 +8,5 @@ plugins:
 
 nav:
   - Home: "index.md"
+  - Other: "other.md"
   - API: "!include api/mkdocs.yml"

__tests__/integration/fixtures/ok-include-path-no-repo-url/api/docs/index.md

@@ -0,0 +1,3 @@
+# API Documentation
+
+Some sub documentation

__tests__/integration/fixtures/ok-include-path-no-repo-url/api/docs/other/other.md

@@ -0,0 +1 @@
+# Other Documentation

__tests__/integration/fixtures/ok-include-path-no-repo-url/api/mkdocs.yml

@@ -0,0 +1,9 @@
+site_name: "test"
+site_description: "This is an api."
+
+plugins:
+  - monorepo
+
+nav:
+  - Home: index.md
+  - Other: other/other.md

__tests__/integration/fixtures/ok-include-path-no-repo-url/docs/index.md

@@ -0,0 +1,3 @@
+# Top Level Project Documentation
+
+Some higher level info about the project

__tests__/integration/fixtures/ok-include-path-no-repo-url/docs/other.md

@@ -0,0 +1 @@
+# Other Documentation

__tests__/integration/fixtures/ok-include-path-no-repo-url/mkdocs.yml

@@ -0,0 +1,10 @@
+site_name: "Example"
+site_description: "Description Here"
+
+plugins:
+  - monorepo
+
+nav:
+  - Home: "index.md"
+  - Other: "other.md"
+  - API: "!include api/mkdocs.yml"

__tests__/integration/fixtures/ok-include-wildcard-edit-uri/docs/index.md

@@ -0,0 +1,3 @@
+# Top Level Project Documentation
+
+Some higher level info about the project

__tests__/integration/fixtures/ok-include-wildcard-edit-uri/docs/other.md

@@ -0,0 +1 @@
+# Other Documentation

__tests__/integration/fixtures/ok-include-wildcard-edit-uri/mkdocs.yml

@@ -0,0 +1,14 @@
+site_name: "Example"
+site_description: "Description Here"
+repo_url: https://github.com/backstage/mkdocs-monorepo-plugin
+edit_uri: edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/docs/
+
+docs_dir: ./docs
+
+plugins:
+  - monorepo
+
+nav:
+  - Home: "index.md"
+  - Other: "other.md"
+  - Projects: "*include ./projects/*/mkdocs.yml"

__tests__/integration/fixtures/ok-include-wildcard-edit-uri/projects/api/docs/index.md

@@ -0,0 +1,3 @@
+# API Documentation
+
+Some sub documentation

__tests__/integration/fixtures/ok-include-wildcard-edit-uri/projects/api/docs/other/other.md

@@ -0,0 +1 @@
+# Other Documentation

__tests__/integration/fixtures/ok-include-wildcard-edit-uri/projects/api/mkdocs.yml

@@ -0,0 +1,11 @@
+site_name: "test"
+site_description: "This is an api."
+repo_url: https://github.com/backstage/mkdocs-monorepo-plugin
+edit_uri: edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/projects/api/docs/
+
+plugins:
+  - monorepo
+
+nav:
+  - Home: index.md
+  - Other: other/other.md

__tests__/integration/fixtures/ok-include-wildcard-no-repo-url/docs/index.md

@@ -0,0 +1,3 @@
+# Top Level Project Documentation
+
+Some higher level info about the project

__tests__/integration/fixtures/ok-include-wildcard-no-repo-url/docs/other.md

@@ -0,0 +1 @@
+# Other Documentation

__tests__/integration/fixtures/ok-include-wildcard-no-repo-url/mkdocs.yml

@@ -0,0 +1,12 @@
+site_name: "Example"
+site_description: "Description Here"
+
+docs_dir: ./docs
+
+plugins:
+  - monorepo
+
+nav:
+  - Home: "index.md"
+  - Other: "other.md"
+  - Projects: "*include ./projects/*/mkdocs.yml"

__tests__/integration/fixtures/ok-include-wildcard-no-repo-url/projects/api/docs/index.md

@@ -0,0 +1,3 @@
+# API Documentation
+
+Some sub documentation

__tests__/integration/fixtures/ok-include-wildcard-no-repo-url/projects/api/docs/other/other.md

@@ -0,0 +1 @@
+# Other Documentation

__tests__/integration/fixtures/ok-include-wildcard-no-repo-url/projects/api/mkdocs.yml

@@ -0,0 +1,9 @@
+site_name: "test"
+site_description: "This is an api."
+
+plugins:
+  - monorepo
+
+nav:
+  - Home: index.md
+  - Other: other/other.md

__tests__/integration/test.bats

@@ -34,6 +34,11 @@ assertFileContains() {
   [ "$status" -eq 0 ]
 }
 
+assertFileNotContains() {
+  run grep $2 $1
+  [ "$status" -eq 1 ]
+}
+
 assertSuccessMkdocs() {
   run mkdocs $@
   debugger
@@ -253,8 +258,8 @@ teardown() {
   [[ "$output" == *"[mkdocs-monorepo] The file path /"*"/__tests__/integration/fixtures/error-include-wildcard-no-site-name/projects/project-a/mkdocs.yml does not contain a valid 'site_name' key in the YAML file. Please include it to indicate where your documentation should be moved to."* ]]
 }
 
-@test "sets edit url for included pages" {
-  cd ${fixturesDir}/bug-include-path-edit-uri
+@test "sets edit url for included path pages" {
+  cd ${fixturesDir}/ok-include-path-edit-uri
   assertSuccessMkdocs build
 
   assertFileContains './site/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/docs/index.md"'
@@ -263,3 +268,37 @@ teardown() {
 
   [ "$status" -eq 0 ]
 }
+
+@test "sets edit url for included wildcard pages" {
+  cd ${fixturesDir}/ok-include-wildcard-edit-uri
+  assertSuccessMkdocs build
+
+  assertFileContains './site/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/docs/index.md"'
+  assertFileContains './site/test/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/projects/api/docs/index.md"'
+  assertFileContains './site/test/other/other/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/projects/api/docs/other/other.md"'
+
+  [ "$status" -eq 0 ]
+}
+
+@test "only set edit_uri for included paths if repo_url is configured" {
+  cd ${fixturesDir}/ok-include-path-no-repo-url
+  assertSuccessMkdocs build
+
+  assertFileNotContains './site/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/docs/index.md"'
+  assertFileNotContains './site/test/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/api/docs/index.md"'
+  assertFileNotContains './site/test/other/other/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/api/docs/other/other.md"'
+
+  [ "$status" -eq 1 ]
+}
+
+@test "only set edit_uri for wildcard paths if repo_url is configured" {
+  cd ${fixturesDir}/ok-include-wildcard-no-repo-url
+  assertSuccessMkdocs build
+
+  assertFileNotContains './site/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/docs/index.md"'
+  assertFileNotContains './site/test/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/projects/api/docs/index.md"'
+  assertFileNotContains './site/test/other/other/index.html' 'href="https://github.com/backstage/mkdocs-monorepo-plugin/edit/fix-edit-page-url/__tests__/integration/fixtures/bug-include-path-edit-uri/projects/api/docs/other/other.md"'
+
+  [ "$status" -eq 1 ]
+}
+

docs/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.5.4
+
+- Fixed edit URLs for included pages
+
 ## 0.5.3
 
 - Don't run on_serve if on_config was skipped

mkdocs_monorepo_plugin/edit_uri.py

@@ -0,0 +1,126 @@
+# Copyright 2022 Spotify AB
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from mkdocs.utils import yaml_load
+from os import path
+
+from urllib.parse import urlsplit
+
+def load_mkdocs_config(file, docs_dir):
+  config = yaml_load(file)
+
+  repo_url = config.get('repo_url')
+  repo_name = config.get('repo_name')
+  edit_uri = config.get('edit_uri')
+
+  # derive repo_name from repo_url if unset
+  if repo_url is not None and repo_name is None:
+    repo_host = urlsplit(repo_url).netloc.lower()
+    if repo_host == 'github.com':
+      repo_name = 'GitHub'
+    elif repo_host == 'bitbucket.org':
+      repo_name = 'Bitbucket'
+    elif repo_host == 'gitlab.com':
+      repo_name = 'GitLab'
+    else:
+      repo_name = repo_host.split('.')[0].title()
+
+  # derive edit_uri from repo_name if unset
+  if repo_name is not None and edit_uri is None:
+    if repo_name == 'GitHub' or repo_name == 'GitLab':
+      edit_uri = 'edit/master/docs/{}'.format(docs_dir)
+    elif repo_name == 'Bitbucket':
+      edit_uri = 'src/default/docs/{}'.format(docs_dir)
+    else:
+      edit_uri = ''
+
+  # ensure a well-formed edit_uri
+  if edit_uri:
+    if not edit_uri.startswith(('?', '#')) \
+      and not config['repo_url'].endswith('/'):
+        config['repo_url'] += '/'
+    if not edit_uri.endswith('/'):
+      edit_uri += '/'
+
+  config['docs_dir'] = docs_dir
+  config['edit_uri'] = edit_uri
+  config['repo_name'] = repo_name
+
+  return config
+
+class EditUrl:
+  def __init__(self, config, page, plugin):
+      self.config = config
+      self.page = page
+      self.plugin = plugin
+  
+  def __get_root_config_file_path(self):
+    return path.dirname(self.config['config_file_path'])
+
+  def __get_root_docs_dir(self):
+    abs_root_config_file_dir = self.__get_root_config_file_path()
+    return path.relpath(self.plugin.originalDocsDir, abs_root_config_file_dir)
+
+  def __get_page_dir_alias(self):
+    parts = self.page.url.split('/')
+    while True:
+      parts.pop()
+      alias = path.join(*parts)
+      if alias in self.plugin.aliases:
+        return alias
+
+  def __get_page_docs_dir(self):
+    alias = self.__get_page_dir_alias()
+    abs_root_config_file_dir = self.__get_root_config_file_path()
+    abs_page_config_file_dir = self.plugin.aliases[alias]['docs_dir']
+    return path.relpath(abs_page_config_file_dir, abs_root_config_file_dir)
+
+  def __get_page_src_path(self):
+    alias = self.page.url.split('/')[0]
+    path = self.page.file.src_path
+    return path.replace('{}/'.format(alias), '')
+
+  def __get_page_config_file_path(self):
+    alias = self.__get_page_dir_alias()
+    return self.plugin.aliases[alias]['yaml_file']
+
+  def __get_page_config_file_yaml(self):
+    page_docs_dir = self.__get_page_docs_dir()
+    abs_page_config_file_path = self.__get_page_config_file_path()
+    with open(abs_page_config_file_path, 'rb') as f:
+      return load_mkdocs_config(f, page_docs_dir)
+
+  def __has_repo(self):
+    page_config_file_yaml = self.__get_page_config_file_yaml()
+    return 'repo_url' in page_config_file_yaml
+
+  def __is_root(self):
+    root_config_docs_dir = self.__get_root_docs_dir()
+    abs_root_config_file_dir = self.__get_root_config_file_path()
+    abs_root_config_docs_dir = path.join(abs_root_config_file_dir, root_config_docs_dir)
+    return abs_root_config_docs_dir in self.page.file.abs_src_path
+
+  def build(self):
+    if self.__is_root():
+      return self.page.edit_url
+    if self.__has_repo():
+      config = self.__get_page_config_file_yaml()
+      self.config['repo_name'] = config['repo_name']
+      return config['repo_url'] + config['edit_uri'] + self.__get_page_src_path()
+    return ''
+    
+def set_edit_url(config, page, plugin):
+  edit_url = EditUrl(config, page, plugin)
+  page.edit_url = edit_url.build()
+

mkdocs_monorepo_plugin/helpers.py

@@ -1,89 +0,0 @@
-# Copyright 2022 Spotify AB
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-from mkdocs.utils import yaml_load
-from os import path, sep
-
-def normalize(path):
-  normalized = path
-  if path.startswith('/'):
-    normalized = normalized[1:]
-  if not path.endswith('/'):
-    normalized = '{}/'.format(path)
-  return normalized
-
-def get_root_config_dir_head(config, plugin):
-  abs_root_config_file_dir = path.dirname(config['config_file_path'])
-  return path.relpath(plugin.originalDocsDir, abs_root_config_file_dir)
-
-def get_page_config_dir_head(config, page):
-  abs_root_config_file_dir = path.dirname(config['config_file_path'])
-  rel_page_markdown_file_path = path.relpath(page.file.abs_src_path, abs_root_config_file_dir)
-  return rel_page_markdown_file_path.split(sep)[0]
-
-def get_page_config_file_dir(config, page):
-  abs_root_config_file_dir = path.dirname(config['config_file_path'])
-  page_config_dir_head = get_page_config_dir_head(config, page)
-  return path.join(abs_root_config_file_dir, page_config_dir_head)
-
-def get_page_config_file_path(config, page):
-  abs_page_config_file_dir = get_page_config_file_dir(config, page)
-  abs_page_config_file_path = path.join(abs_page_config_file_dir, 'mkdocs.yaml')
-  if not path.exists(abs_page_config_file_path):
-    abs_page_config_file_path = path.join(abs_page_config_file_dir, 'mkdocs.yml')
-  return abs_page_config_file_path
-
-def get_page_config_file_yaml(config, page):
-  abs_page_config_file_path = get_page_config_file_path(config, page)
-  with open(abs_page_config_file_path, 'rb') as f:
-    return { 'docs_dir': 'docs', **yaml_load(f) }
-
-def replace_page_repo_url(config, page):
-  page_config_file_yaml = get_page_config_file_yaml(config, page)
-  if 'repo_url' in page_config_file_yaml:
-    page_config_repo_url = normalize(page_config_file_yaml['repo_url'])
-    page.edit_url = page.edit_url.replace(config['repo_url'], page_config_repo_url)
-
-def replace_page_docs_dir(config, page, plugin):
-  root_config_dir_head = get_root_config_dir_head(config, plugin)
-  page_config_dir_head = get_page_config_dir_head(config, page)
-  page_config_file_yaml = get_page_config_file_yaml(config, page)
-  page_config_docs_dir = page_config_file_yaml['docs_dir']
-
-  root_docs_dir_text = '/{}/'.format(root_config_dir_head)
-  page_docs_dir_text = '/{}/{}/'.format(page_config_dir_head, page_config_docs_dir)
-
-  page.edit_url = page.edit_url.replace(root_docs_dir_text, page_docs_dir_text)
-
-def replace_page_edit_uri(config, page, plugin):
-  page_config_file_yaml = get_page_config_file_yaml(config, page)
-  if 'edit_uri' in page_config_file_yaml:
-    page_config_edit_uri = normalize(page_config_file_yaml['edit_uri'])
-    page.edit_url = page.edit_url.replace(config['edit_uri'], page_config_edit_uri)
-  else:
-    replace_page_docs_dir(config, page, plugin)
-
-def replace_page_dir_alias(page):
-  page_docs_dir_alias = page.url.split('/')[0]
-  page_docs_dir_alias_text = '/{}'.format(page_docs_dir_alias)
-  page.edit_url = page.edit_url.replace(page_docs_dir_alias_text, '')
-
-def is_index(page):
-  return len(page.url) == 0
-
-def set_edit_url(config, page, plugin):
-  if page.edit_url is not None:
-    replace_page_dir_alias(page)
-    replace_page_repo_url(config, page)
-    replace_page_edit_uri(config, page, plugin)

mkdocs_monorepo_plugin/plugin.py

@@ -15,7 +15,7 @@
 from mkdocs.plugins import BasePlugin
 from .parser import Parser
 from .merger import Merger
-from .helpers import is_index, set_edit_url
+from .edit_uri import set_edit_url
 
 
 class MonorepoPlugin(BasePlugin):
@@ -45,7 +45,9 @@ class MonorepoPlugin(BasePlugin):
 
         # Generate a new "docs" directory
         self.merger = Merger(config)
+        self.aliases = {}
         for alias, docs_dir, yaml_file in resolvedPaths:
+            self.aliases[alias] = { 'docs_dir': docs_dir, 'yaml_file': yaml_file }
             self.merger.append(alias, docs_dir)
         new_docs_dir = self.merger.merge()
 
@@ -66,8 +68,7 @@ class MonorepoPlugin(BasePlugin):
         if len(self.files_source_dir) > 0:
             if page.file.abs_src_path in self.files_source_dir:
                 page.file.abs_src_path = self.files_source_dir[page.file.abs_src_path]
-                if not is_index(page):
-                    set_edit_url(config, page, self)
+                set_edit_url(config, page, self)
         return page
 
     def on_serve(self, server, config, **kwargs):