From bf7c4300ceb6a0a11d62f2003e9bc043d4b8a16b Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 30 Jun 2023 15:10:16 +0200 Subject: [PATCH 1/7] Refactor to eliminate wrapper Markdown extension classes --- mkdocs/structure/pages.py | 57 ++++++++++++++------------------------- 1 file changed, 20 insertions(+), 37 deletions(-) diff --git a/mkdocs/structure/pages.py b/mkdocs/structure/pages.py index 5f2676af..4ee36aca 100644 --- a/mkdocs/structure/pages.py +++ b/mkdocs/structure/pages.py @@ -263,19 +263,20 @@ class Page(StructureItem): if self.markdown is None: raise RuntimeError("`markdown` field hasn't been set (via `read_source`)") - relative_path_extension = _RelativePathExtension(self.file, files) - extract_title_extension = _ExtractTitleExtension() md = markdown.Markdown( - extensions=[ - relative_path_extension, - extract_title_extension, - *config['markdown_extensions'], - ], + extensions=config['markdown_extensions'], extension_configs=config['mdx_configs'] or {}, ) + + relative_path_ext = _RelativePathTreeprocessor(self.file, files) + relative_path_ext._register(md) + + extract_title_ext = _ExtractTitleTreeprocessor() + extract_title_ext._register(md) + self.content = md.convert(self.markdown) self.toc = get_toc(getattr(md, 'toc_tokens', [])) - self._title_from_render = extract_title_extension.title + self._title_from_render = extract_title_ext.title class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): @@ -343,37 +344,12 @@ class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): components = (scheme, netloc, path, query, fragment) return urlunsplit(components) - -class _RelativePathExtension(markdown.extensions.Extension): - """ - The Extension class is what we pass to markdown, it then - registers the Treeprocessor. - """ - - def __init__(self, file: File, files: Files) -> None: - self.file = file - self.files = files - - def extendMarkdown(self, md: markdown.Markdown) -> None: - relpath = _RelativePathTreeprocessor(self.file, self.files) - md.treeprocessors.register(relpath, "relpath", 0) - - -class _ExtractTitleExtension(markdown.extensions.Extension): - def __init__(self) -> None: - self.title: str | None = None - - def extendMarkdown(self, md: markdown.Markdown) -> None: - md.treeprocessors.register( - _ExtractTitleTreeprocessor(self), - "mkdocs_extract_title", - priority=1, # Close to the end. - ) + def _register(self, md: markdown.Markdown) -> None: + md.treeprocessors.register(self, "relpath", 0) class _ExtractTitleTreeprocessor(markdown.treeprocessors.Treeprocessor): - def __init__(self, ext: _ExtractTitleExtension) -> None: - self.ext = ext + title: str | None = None def run(self, root: etree.Element) -> etree.Element: for el in root: @@ -383,6 +359,13 @@ class _ExtractTitleTreeprocessor(markdown.treeprocessors.Treeprocessor): el = copy.copy(el) del el[-1] # Extract the text only, recursively. - self.ext.title = _unescape(''.join(el.itertext())) + self.title = _unescape(''.join(el.itertext())) break return root + + def _register(self, md: markdown.Markdown) -> None: + md.treeprocessors.register( + self, + "mkdocs_extract_title", + priority=1, # Close to the end. + ) From 5db6a7808229900d183ff1dcaec82f0a2df449bc Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 30 Jun 2023 15:47:36 +0200 Subject: [PATCH 2/7] Deprecate `defaults.get_schema` --- mkdocs/config/defaults.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mkdocs/config/defaults.py b/mkdocs/config/defaults.py index 1a76f1fa..cf6e630c 100644 --- a/mkdocs/config/defaults.py +++ b/mkdocs/config/defaults.py @@ -8,10 +8,6 @@ from mkdocs.config import config_options as c from mkdocs.utils.yaml import get_yaml_loader, yaml_load -def get_schema() -> base.PlainConfigSchema: - return MkDocsConfig._schema - - # NOTE: The order here is important. During validation some config options # depend on others. So, if config option A depends on B, then A should be # listed higher in the schema. @@ -152,3 +148,8 @@ class MkDocsConfig(base.Config): """Load config options from the open file descriptor of a YAML file.""" loader = get_yaml_loader(config=self) self.load_dict(yaml_load(config_file, loader)) + + +def get_schema() -> base.PlainConfigSchema: + """Soft-deprecated, do not use.""" + return MkDocsConfig._schema From c71bf2429fc97c964fab13f1220feadd40f383b1 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 1 Jul 2023 13:37:28 +0200 Subject: [PATCH 3/7] Refactor RelativePathExtensionTests And use attribute access for configs in more places --- mkdocs/tests/build_tests.py | 84 ++++---- mkdocs/tests/config/base_tests.py | 16 +- mkdocs/tests/search_tests.py | 14 +- mkdocs/tests/structure/nav_tests.py | 64 ++----- mkdocs/tests/structure/page_tests.py | 275 ++++++++++++++------------- 5 files changed, 206 insertions(+), 247 deletions(-) diff --git a/mkdocs/tests/build_tests.py b/mkdocs/tests/build_tests.py index 24b2fa6c..8d0eb0fa 100644 --- a/mkdocs/tests/build_tests.py +++ b/mkdocs/tests/build_tests.py @@ -58,9 +58,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): {'Home': 'index.md'}, ] cfg = load_config(nav=nav_cfg, use_directory_urls=False) - fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - ] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) context = build.get_context(nav, files, cfg, nav.pages[0]) @@ -71,9 +69,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): {'Home': 'index.md'}, ] cfg = load_config(nav=nav_cfg) - fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - ] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) context = build.get_context(nav, files, cfg, nav.pages[0]) @@ -86,8 +82,8 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): ] cfg = load_config(nav=nav_cfg, use_directory_urls=False) fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - File('foo/bar.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), + File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), + File('foo/bar.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), ] files = Files(fs) nav = get_navigation(files, cfg) @@ -101,8 +97,8 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): ] cfg = load_config(nav=nav_cfg) fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - File('foo/bar.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), + File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), + File('foo/bar.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), ] files = Files(fs) nav = get_navigation(files, cfg) @@ -149,9 +145,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): extra_javascript=['script.js'], use_directory_urls=False, ) - fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - ] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) context = build.get_context(nav, files, cfg, nav.pages[0]) @@ -170,8 +164,8 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): use_directory_urls=False, ) fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - File('foo/bar.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), + File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), + File('foo/bar.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), ] files = Files(fs) nav = get_navigation(files, cfg) @@ -190,8 +184,8 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): extra_javascript=['script.js'], ) fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - File('foo/bar.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), + File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), + File('foo/bar.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), ] files = Files(fs) nav = get_navigation(files, cfg) @@ -210,9 +204,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): extra_css=['assets\\style.css'], use_directory_urls=False, ) - fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - ] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) with self.assertLogs('mkdocs') as cm: @@ -241,7 +233,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.commands.build._build_template', return_value='some content') def test_build_theme_template(self, mock_build_template, mock_write_file): cfg = load_config() - env = cfg['theme'].get_env() + env = cfg.theme.get_env() build._build_theme_template('main.html', env, mock.Mock(), cfg, mock.Mock()) mock_write_file.assert_called_once() mock_build_template.assert_called_once() @@ -254,7 +246,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): self, site_dir, mock_gzip_gzipfile, mock_build_template, mock_write_file ): cfg = load_config(site_dir=site_dir) - env = cfg['theme'].get_env() + env = cfg.theme.get_env() build._build_theme_template('sitemap.xml', env, mock.Mock(), cfg, mock.Mock()) mock_write_file.assert_called_once() mock_build_template.assert_called_once() @@ -264,7 +256,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.commands.build._build_template', return_value='') def test_skip_missing_theme_template(self, mock_build_template, mock_write_file): cfg = load_config() - env = cfg['theme'].get_env() + env = cfg.theme.get_env() with self.assertLogs('mkdocs') as cm: build._build_theme_template('missing.html', env, mock.Mock(), cfg, mock.Mock()) self.assertEqual( @@ -278,7 +270,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.commands.build._build_template', return_value='') def test_skip_theme_template_empty_output(self, mock_build_template, mock_write_file): cfg = load_config() - env = cfg['theme'].get_env() + env = cfg.theme.get_env() with self.assertLogs('mkdocs') as cm: build._build_theme_template('main.html', env, mock.Mock(), cfg, mock.Mock()) self.assertEqual( @@ -294,18 +286,14 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.commands.build.open', mock.mock_open(read_data='template content')) def test_build_extra_template(self, site_dir): cfg = load_config(site_dir=site_dir) - fs = [ - File('foo.html', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - ] + fs = [File('foo.html', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) build._build_extra_template('foo.html', files, cfg, mock.Mock()) @mock.patch('mkdocs.commands.build.open', mock.mock_open(read_data='template content')) def test_skip_missing_extra_template(self): cfg = load_config() - fs = [ - File('foo.html', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - ] + fs = [File('foo.html', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) with self.assertLogs('mkdocs') as cm: build._build_extra_template('missing.html', files, cfg, mock.Mock()) @@ -317,9 +305,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.commands.build.open', side_effect=OSError('Error message.')) def test_skip_ioerror_extra_template(self, mock_open): cfg = load_config() - fs = [ - File('foo.html', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - ] + fs = [File('foo.html', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) with self.assertLogs('mkdocs') as cm: build._build_extra_template('foo.html', files, cfg, mock.Mock()) @@ -331,9 +317,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.commands.build.open', mock.mock_open(read_data='')) def test_skip_extra_template_empty_output(self): cfg = load_config() - fs = [ - File('foo.html', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - ] + fs = [File('foo.html', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) with self.assertLogs('mkdocs') as cm: build._build_extra_template('foo.html', files, cfg, mock.Mock()) @@ -347,7 +331,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @tempdir(files={'index.md': 'page content'}) def test_populate_page(self, docs_dir): cfg = load_config(docs_dir=docs_dir) - file = File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + file = File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) page = Page('Foo', file, cfg) build._populate_page(page, cfg, Files([file])) self.assertEqual(page.content, '

page content

') @@ -355,7 +339,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @tempdir(files={'testing.html': '

page content

'}) def test_populate_page_dirty_modified(self, site_dir): cfg = load_config(site_dir=site_dir) - file = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + file = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) page = Page('Foo', file, cfg) build._populate_page(page, cfg, Files([file]), dirty=True) self.assertTrue(page.markdown.startswith('# Welcome to MkDocs')) @@ -367,7 +351,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @tempdir(files={'index.html': '

page content

'}) def test_populate_page_dirty_not_modified(self, site_dir, docs_dir): cfg = load_config(docs_dir=docs_dir, site_dir=site_dir) - file = File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + file = File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) page = Page('Foo', file, cfg) build._populate_page(page, cfg, Files([file]), dirty=True) # Content is empty as file read was skipped @@ -378,7 +362,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.structure.pages.open', side_effect=OSError('Error message.')) def test_populate_page_read_error(self, docs_dir, mock_open): cfg = load_config(docs_dir=docs_dir) - file = File('missing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + file = File('missing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) page = Page('Foo', file, cfg) with self.assertLogs('mkdocs') as cm: with self.assertRaises(OSError): @@ -400,7 +384,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): cfg = load_config(docs_dir=docs_dir) cfg.plugins.events['page_markdown'].append(on_page_markdown) - file = File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + file = File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) page = Page('Foo', file, cfg) with self.assertLogs('mkdocs') as cm: with self.assertRaises(PluginError): @@ -415,7 +399,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @tempdir() def test_build_page(self, site_dir): cfg = load_config(site_dir=site_dir, nav=['index.md']) - fs = [File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) page = files.documentation_pages()[0].page @@ -430,12 +414,12 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('jinja2.environment.Template.render', return_value='') def test_build_page_empty(self, site_dir, render_mock): cfg = load_config(site_dir=site_dir, nav=['index.md']) - fs = [File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) with self.assertLogs('mkdocs') as cm: build._build_page( - files.documentation_pages()[0].page, cfg, files, nav, cfg['theme'].get_env() + files.documentation_pages()[0].page, cfg, files, nav, cfg.theme.get_env() ) self.assertEqual( '\n'.join(cm.output), @@ -449,7 +433,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.utils.write_file') def test_build_page_dirty_modified(self, site_dir, docs_dir, mock_write_file): cfg = load_config(docs_dir=docs_dir, site_dir=site_dir, nav=['index.md']) - fs = [File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) page = files.documentation_pages()[0].page @@ -466,7 +450,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.utils.write_file') def test_build_page_dirty_not_modified(self, site_dir, mock_write_file): cfg = load_config(site_dir=site_dir, nav=['testing.md']) - fs = [File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) page = files.documentation_pages()[0].page @@ -482,7 +466,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @tempdir() def test_build_page_custom_template(self, site_dir): cfg = load_config(site_dir=site_dir, nav=['index.md']) - fs = [File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) page = files.documentation_pages()[0].page @@ -498,7 +482,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): @mock.patch('mkdocs.utils.write_file', side_effect=OSError('Error message.')) def test_build_page_error(self, site_dir, mock_write_file): cfg = load_config(site_dir=site_dir, nav=['index.md']) - fs = [File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) page = files.documentation_pages()[0].page @@ -522,7 +506,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): cfg = load_config(site_dir=site_dir, nav=['index.md']) cfg.plugins.events['page_context'].append(on_page_context) - fs = [File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) nav = get_navigation(files, cfg) page = files.documentation_pages()[0].page @@ -532,7 +516,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): page.content = '

page content

' with self.assertLogs('mkdocs') as cm: with self.assertRaises(PluginError): - build._build_page(page, cfg, files, nav, cfg['theme'].get_env()) + build._build_page(page, cfg, files, nav, cfg.theme.get_env()) self.assertEqual( '\n'.join(cm.output), "ERROR:mkdocs.commands.build:Error building page 'index.md':", diff --git a/mkdocs/tests/config/base_tests.py b/mkdocs/tests/config/base_tests.py index 1a2a85a3..2590b718 100644 --- a/mkdocs/tests/config/base_tests.py +++ b/mkdocs/tests/config/base_tests.py @@ -52,7 +52,7 @@ class ConfigBaseTests(unittest.TestCase): cfg = base.load_config(config_file=config_file.name) self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) - self.assertEqual(cfg['site_name'], 'MkDocs Test') + self.assertEqual(cfg.site_name, 'MkDocs Test') @tempdir() def test_load_default_file(self, temp_dir): @@ -65,7 +65,7 @@ class ConfigBaseTests(unittest.TestCase): with change_dir(temp_dir): cfg = base.load_config(config_file=None) self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) - self.assertEqual(cfg['site_name'], 'MkDocs Test') + self.assertEqual(cfg.site_name, 'MkDocs Test') @tempdir def test_load_default_file_with_yaml(self, temp_dir): @@ -78,7 +78,7 @@ class ConfigBaseTests(unittest.TestCase): with change_dir(temp_dir): cfg = base.load_config(config_file=None) self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) - self.assertEqual(cfg['site_name'], 'MkDocs Test') + self.assertEqual(cfg.site_name, 'MkDocs Test') @tempdir() def test_load_default_file_prefer_yml(self, temp_dir): @@ -94,7 +94,7 @@ class ConfigBaseTests(unittest.TestCase): with change_dir(temp_dir): cfg = base.load_config(config_file=None) self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) - self.assertEqual(cfg['site_name'], 'MkDocs Test1') + self.assertEqual(cfg.site_name, 'MkDocs Test1') def test_load_from_missing_file(self): with self.assertRaisesRegex( @@ -115,7 +115,7 @@ class ConfigBaseTests(unittest.TestCase): cfg = base.load_config(config_file=config_file) self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) - self.assertEqual(cfg['site_name'], 'MkDocs Test') + self.assertEqual(cfg.site_name, 'MkDocs Test') # load_config will always close the file self.assertTrue(config_file.closed) @@ -131,7 +131,7 @@ class ConfigBaseTests(unittest.TestCase): cfg = base.load_config(config_file=config_file) self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) - self.assertEqual(cfg['site_name'], 'MkDocs Test') + self.assertEqual(cfg.site_name, 'MkDocs Test') @tempdir def test_load_missing_required(self, temp_dir): @@ -265,8 +265,8 @@ class ConfigBaseTests(unittest.TestCase): cfg = base.load_config(config_file=config_file) self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) - self.assertEqual(cfg['site_name'], 'MkDocs Test') - self.assertEqual(cfg['docs_dir'], docs_dir) + self.assertEqual(cfg.site_name, 'MkDocs Test') + self.assertEqual(cfg.docs_dir, docs_dir) self.assertEqual(cfg.config_file_path, config_fname) self.assertIsInstance(cfg.config_file_path, str) diff --git a/mkdocs/tests/search_tests.py b/mkdocs/tests/search_tests.py index dd8acb24..ea176fca 100644 --- a/mkdocs/tests/search_tests.py +++ b/mkdocs/tests/search_tests.py @@ -354,22 +354,12 @@ class SearchIndexTests(unittest.TestCase): pages = [ Page( 'Home', - File( - 'index.md', - base_cfg['docs_dir'], - base_cfg['site_dir'], - base_cfg['use_directory_urls'], - ), + File('index.md', base_cfg.docs_dir, base_cfg.site_dir, base_cfg.use_directory_urls), base_cfg, ), Page( 'About', - File( - 'about.md', - base_cfg['docs_dir'], - base_cfg['site_dir'], - base_cfg['use_directory_urls'], - ), + File('about.md', base_cfg.docs_dir, base_cfg.site_dir, base_cfg.use_directory_urls), base_cfg, ), ] diff --git a/mkdocs/tests/structure/nav_tests.py b/mkdocs/tests/structure/nav_tests.py index 5751429e..1ef971f6 100644 --- a/mkdocs/tests/structure/nav_tests.py +++ b/mkdocs/tests/structure/nav_tests.py @@ -25,9 +25,7 @@ class SiteNavigationTests(unittest.TestCase): ) cfg = load_config(nav=nav_cfg, site_url='http://example.com/') fs = [ - File( - list(item.values())[0], cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'] - ) + File(list(item.values())[0], cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for item in nav_cfg ] files = Files(fs) @@ -50,9 +48,7 @@ class SiteNavigationTests(unittest.TestCase): ) cfg = load_config(nav=nav_cfg, use_directory_urls=False, site_url='http://example.com/') fs = [ - File( - list(item.values())[0], cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'] - ) + File(list(item.values())[0], cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for item in nav_cfg ] files = Files(fs) @@ -73,8 +69,8 @@ class SiteNavigationTests(unittest.TestCase): ) cfg = load_config(nav=nav_cfg, site_url='http://example.com/') fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - File('page_not_in_nav.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), + File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), + File('page_not_in_nav.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), ] files = Files(fs) site_navigation = get_navigation(files, cfg) @@ -97,8 +93,8 @@ class SiteNavigationTests(unittest.TestCase): ) cfg = load_config(nav=nav_cfg, site_url='http://example.com/') fs = [ - File(nav_cfg[0], cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - File(nav_cfg[1]['About'], cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), + File(nav_cfg[0], cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), + File(nav_cfg[1]['About'], cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), ] files = Files(fs) site_navigation = get_navigation(files, cfg) @@ -120,17 +116,15 @@ class SiteNavigationTests(unittest.TestCase): """ ) cfg = load_config(nav=nav_cfg, site_url='http://example.com/') - fs = [File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) with self.assertLogs('mkdocs', level='DEBUG') as cm: site_navigation = get_navigation(files, cfg) self.assertEqual( cm.output, [ - "DEBUG:mkdocs.structure.nav:An absolute path to '/local.html' is included in the " - "'nav' configuration, which presumably points to an external resource.", - "DEBUG:mkdocs.structure.nav:An external link to 'http://example.com/external.html' " - "is included in the 'nav' configuration.", + "DEBUG:mkdocs.structure.nav:An absolute path to '/local.html' is included in the 'nav' configuration, which presumably points to an external resource.", + "DEBUG:mkdocs.structure.nav:An external link to 'http://example.com/external.html' is included in the 'nav' configuration.", ], ) self.assertEqual(str(site_navigation).strip(), expected) @@ -151,17 +145,15 @@ class SiteNavigationTests(unittest.TestCase): """ ) cfg = load_config(nav=nav_cfg, site_url='http://example.com/') - fs = [File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) with self.assertLogs('mkdocs') as cm: site_navigation = get_navigation(files, cfg) self.assertEqual( cm.output, [ - "WARNING:mkdocs.structure.nav:A relative path to 'missing.html' is included " - "in the 'nav' configuration, which is not found in the documentation files.", - "WARNING:mkdocs.structure.nav:A relative path to 'example.com' is included " - "in the 'nav' configuration, which is not found in the documentation files.", + "WARNING:mkdocs.structure.nav:A relative path to 'missing.html' is included in the 'nav' configuration, which is not found in the documentation files.", + "WARNING:mkdocs.structure.nav:A relative path to 'example.com' is included in the 'nav' configuration, which is not found in the documentation files.", ], ) self.assertEqual(str(site_navigation).strip(), expected) @@ -215,9 +207,7 @@ class SiteNavigationTests(unittest.TestCase): 'api-guide/advanced/part-1.md', 'about/release-notes.md', ] - files = Files( - [File(s, cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) for s in fs] - ) + files = Files([File(s, cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for s in fs]) site_navigation = get_navigation(files, cfg) self.assertEqual(str(site_navigation).strip(), expected) self.assertEqual(len(site_navigation.items), 4) @@ -282,9 +272,7 @@ class SiteNavigationTests(unittest.TestCase): ) cfg = load_config(nav=nav_cfg, site_url='http://example.com/') fs = [ - File( - list(item.values())[0], cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'] - ) + File(list(item.values())[0], cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for item in nav_cfg ] files = Files(fs) @@ -308,10 +296,7 @@ class SiteNavigationTests(unittest.TestCase): ) cfg = load_config(nav=nav_cfg, site_url='http://example.com/') - fs = [ - File(item, cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) - for item in nav_cfg - ] + fs = [File(item, cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for item in nav_cfg] files = Files(fs) site_navigation = get_navigation(files, cfg) self.assertEqual(str(site_navigation).strip(), expected) @@ -335,10 +320,7 @@ class SiteNavigationTests(unittest.TestCase): ) cfg = load_config(nav=nav_cfg, site_url='http://example.com/') - fs = [ - File(item, cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) - for item in nav_cfg - ] + fs = [File(item, cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for item in nav_cfg] files = Files(fs) site_navigation = get_navigation(files, cfg) self.assertEqual(str(site_navigation).strip(), expected) @@ -354,8 +336,8 @@ class SiteNavigationTests(unittest.TestCase): ) cfg = load_config(site_url='http://example.com/') fs = [ - File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), - File('about.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']), + File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), + File('about.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls), ] files = Files(fs) site_navigation = get_navigation(files, cfg) @@ -389,9 +371,7 @@ class SiteNavigationTests(unittest.TestCase): 'api-guide/testing.md', 'api-guide/advanced/part-1.md', ] - files = Files( - [File(s, cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) for s in fs] - ) + files = Files([File(s, cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for s in fs]) site_navigation = get_navigation(files, cfg) self.assertEqual(str(site_navigation).strip(), expected) self.assertEqual(len(site_navigation.items), 3) @@ -430,9 +410,7 @@ class SiteNavigationTests(unittest.TestCase): 'about/release-notes.md', 'about/license.md', ] - files = Files( - [File(s, cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) for s in fs] - ) + files = Files([File(s, cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for s in fs]) site_navigation = get_navigation(files, cfg) # Confirm nothing is active self.assertTrue(all(page.active is False for page in site_navigation.pages)) @@ -471,7 +449,7 @@ class SiteNavigationTests(unittest.TestCase): }, ] cfg = load_config(nav=nav_cfg, site_url='http://example.com/') - fs = [File('page.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'])] + fs = [File('page.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls)] files = Files(fs) site_navigation = get_navigation(files, cfg) self.assertEqual(len(_get_by_type(site_navigation, Section)), 2) diff --git a/mkdocs/tests/structure/page_tests.py b/mkdocs/tests/structure/page_tests.py index 30a90f68..7d5160f3 100644 --- a/mkdocs/tests/structure/page_tests.py +++ b/mkdocs/tests/structure/page_tests.py @@ -1,6 +1,9 @@ +from __future__ import annotations + import functools import os import sys +import textwrap import unittest from unittest import mock @@ -11,15 +14,13 @@ from mkdocs.tests.base import dedent, load_config, tempdir load_config = functools.lru_cache(maxsize=None)(load_config) +DOCS_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), '../integration/subpages/docs') + class PageTests(unittest.TestCase): - DOCS_DIR = os.path.join( - os.path.abspath(os.path.dirname(__file__)), '../integration/subpages/docs' - ) - def test_homepage(self): - cfg = load_config(docs_dir=self.DOCS_DIR) - fl = File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR) + fl = File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) self.assertIsNone(fl.page) pg = Page('Foo', fl, cfg) self.assertEqual(fl.page, pg) @@ -43,8 +44,8 @@ class PageTests(unittest.TestCase): self.assertEqual(pg.toc, []) def test_nested_index_page(self): - cfg = load_config(docs_dir=self.DOCS_DIR) - fl = File('sub1/index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR) + fl = File('sub1/index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) pg.parent = 'foo' self.assertEqual(pg.url, 'sub1/') @@ -67,8 +68,8 @@ class PageTests(unittest.TestCase): self.assertEqual(pg.toc, []) def test_nested_index_page_no_parent(self): - cfg = load_config(docs_dir=self.DOCS_DIR) - fl = File('sub1/index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR) + fl = File('sub1/index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) pg.parent = None # non-homepage at nav root level; see #1919. self.assertEqual(pg.url, 'sub1/') @@ -91,8 +92,8 @@ class PageTests(unittest.TestCase): self.assertEqual(pg.toc, []) def test_nested_index_page_no_parent_no_directory_urls(self): - cfg = load_config(docs_dir=self.DOCS_DIR, use_directory_urls=False) - fl = File('sub1/index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR, use_directory_urls=False) + fl = File('sub1/index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) pg.parent = None # non-homepage at nav root level; see #1919. self.assertEqual(pg.url, 'sub1/index.html') @@ -115,8 +116,8 @@ class PageTests(unittest.TestCase): self.assertEqual(pg.toc, []) def test_nested_nonindex_page(self): - cfg = load_config(docs_dir=self.DOCS_DIR) - fl = File('sub1/non-index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR) + fl = File('sub1/non-index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) pg.parent = 'foo' self.assertEqual(pg.url, 'sub1/non-index/') @@ -140,7 +141,7 @@ class PageTests(unittest.TestCase): def test_page_defaults(self): cfg = load_config() - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertRegex(pg.update_date, r'\d{4}-\d{2}-\d{2}') self.assertEqual(pg.url, 'testing/') @@ -164,7 +165,7 @@ class PageTests(unittest.TestCase): def test_page_no_directory_url(self): cfg = load_config(use_directory_urls=False) - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertEqual(pg.url, 'testing.html') self.assertEqual(pg.abs_url, None) @@ -187,7 +188,7 @@ class PageTests(unittest.TestCase): def test_page_canonical_url(self): cfg = load_config(site_url='http://example.com') - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertEqual(pg.url, 'testing/') self.assertEqual(pg.abs_url, '/testing/') @@ -210,7 +211,7 @@ class PageTests(unittest.TestCase): def test_page_canonical_url_nested(self): cfg = load_config(site_url='http://example.com/foo/') - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertEqual(pg.url, 'testing/') self.assertEqual(pg.abs_url, '/foo/testing/') @@ -233,7 +234,7 @@ class PageTests(unittest.TestCase): def test_page_canonical_url_nested_no_slash(self): cfg = load_config(site_url='http://example.com/foo') - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertEqual(pg.url, 'testing/') self.assertEqual(pg.abs_url, '/foo/testing/') @@ -256,7 +257,7 @@ class PageTests(unittest.TestCase): def test_predefined_page_title(self): cfg = load_config() - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Page Title', fl, cfg) pg.read_source(cfg) self.assertEqual(pg.url, 'testing/') @@ -280,7 +281,7 @@ class PageTests(unittest.TestCase): def test_page_title_from_markdown(self): cfg = load_config() - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page(None, fl, cfg) pg.read_source(cfg) self.assertEqual(pg.url, 'testing/') @@ -380,8 +381,8 @@ class PageTests(unittest.TestCase): self.assertEqual(pg.title, 'Welcome to MkDocs Attr { #welcome }') def test_page_title_from_meta(self): - cfg = load_config(docs_dir=self.DOCS_DIR) - fl = File('metadata.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR) + fl = File('metadata.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page(None, fl, cfg) pg.read_source(cfg) self.assertEqual(pg.url, 'metadata/') @@ -406,8 +407,8 @@ class PageTests(unittest.TestCase): self.assertEqual(pg.title, 'A Page Title') def test_page_title_from_filename(self): - cfg = load_config(docs_dir=self.DOCS_DIR) - fl = File('page-title.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR) + fl = File('page-title.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page(None, fl, cfg) pg.read_source(cfg) self.assertEqual(pg.url, 'page-title/') @@ -431,8 +432,8 @@ class PageTests(unittest.TestCase): self.assertEqual(pg.title, 'Page title') def test_page_title_from_capitalized_filename(self): - cfg = load_config(docs_dir=self.DOCS_DIR) - fl = File('pageTitle.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR) + fl = File('pageTitle.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page(None, fl, cfg) pg.read_source(cfg) self.assertEqual(pg.url, 'pageTitle/') @@ -454,8 +455,8 @@ class PageTests(unittest.TestCase): self.assertEqual(pg.title, 'pageTitle') def test_page_title_from_homepage_filename(self): - cfg = load_config(docs_dir=self.DOCS_DIR) - fl = File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + cfg = load_config(docs_dir=DOCS_DIR) + fl = File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page(None, fl, cfg) pg.read_source(cfg) self.assertEqual(pg.url, '') @@ -479,14 +480,14 @@ class PageTests(unittest.TestCase): def test_page_eq(self): cfg = load_config() - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertTrue(pg == Page('Foo', fl, cfg)) def test_page_ne(self): cfg = load_config() - f1 = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) - f2 = File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + f1 = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) + f2 = File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', f1, cfg) # Different Title self.assertTrue(pg != Page('Bar', f1, cfg)) @@ -497,7 +498,7 @@ class PageTests(unittest.TestCase): def test_BOM(self, docs_dir): md_src = '# An UTF-8 encoded file with a BOM' cfg = load_config(docs_dir=docs_dir) - fl = File('index.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('index.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page(None, fl, cfg) # Create an UTF-8 Encoded file with BOM (as Microsoft editors do). See #1186 with open(fl.abs_src_path, 'w', encoding='utf-8-sig') as f: @@ -632,7 +633,7 @@ class PageTests(unittest.TestCase): edit_url_key = f'edit_url{i}' if i > 1 else 'edit_url' with self.subTest(case['config'], path=path): cfg = load_config(**case['config']) - fl = File(path, cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File(path, cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertEqual(pg.url, paths[path]) self.assertEqual(pg.edit_url, case[edit_url_key]) @@ -655,9 +656,7 @@ class PageTests(unittest.TestCase): with self.subTest(case['config']): with self.assertLogs('mkdocs') as cm: cfg = load_config(**case['config']) - fl = File( - 'testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls'] - ) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertEqual(pg.url, 'testing/') self.assertEqual(pg.edit_url, case['edit_url']) @@ -665,7 +664,7 @@ class PageTests(unittest.TestCase): def test_page_render(self): cfg = load_config() - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) pg.read_source(cfg) self.assertEqual(pg.content, None) @@ -687,7 +686,7 @@ class PageTests(unittest.TestCase): def test_missing_page(self): cfg = load_config() - fl = File('missing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('missing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) with self.assertLogs('mkdocs') as cm: with self.assertRaises(OSError): @@ -704,7 +703,7 @@ class SourceDateEpochTests(unittest.TestCase): def test_source_date_epoch(self): cfg = load_config() - fl = File('testing.md', cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) + fl = File('testing.md', cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) pg = Page('Foo', fl, cfg) self.assertEqual(pg.update_date, '1970-01-01') @@ -716,180 +715,188 @@ class SourceDateEpochTests(unittest.TestCase): class RelativePathExtensionTests(unittest.TestCase): - DOCS_DIR = os.path.join( - os.path.abspath(os.path.dirname(__file__)), '../integration/subpages/docs' - ) - - def get_rendered_result(self, files): - cfg = load_config(docs_dir=self.DOCS_DIR) - fs = [File(f, cfg['docs_dir'], cfg['site_dir'], cfg['use_directory_urls']) for f in files] + def get_rendered_result( + self, *, content: str, files: list[str], logs: str = '', **kwargs + ) -> str: + cfg = load_config(docs_dir=DOCS_DIR, **kwargs) + fs = [File(f, cfg.docs_dir, cfg.site_dir, cfg.use_directory_urls) for f in files] pg = Page('Foo', fs[0], cfg) - pg.read_source(cfg) - pg.render(cfg, Files(fs)) - return pg.content - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='[link](non-index.md)')) + with mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data=content)): + pg.read_source(cfg) + if logs: + with self.assertLogs('mkdocs.structure.pages') as cm: + pg.render(cfg, Files(fs)) + msgs = [f'{r.levelname}:{r.message}' for r in cm.records] + self.assertEqual('\n'.join(msgs), textwrap.dedent(logs).strip('\n')) + elif sys.version_info >= (3, 10): + with self.assertNoLogs('mkdocs.structure.pages'): + pg.render(cfg, Files(fs)) + else: + pg.render(cfg, Files(fs)) + + assert pg.content is not None + content = pg.content + if content.startswith('

') and content.endswith('

'): + content = content[3:-4] + return content + def test_relative_html_link(self): self.assertEqual( - self.get_rendered_result(['index.md', 'non-index.md']), - '

link

', # No trailing / + self.get_rendered_result( + content='[link](non-index.md)', files=['index.md', 'non-index.md'] + ), + 'link', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='[link](index.md)')) def test_relative_html_link_index(self): self.assertEqual( - self.get_rendered_result(['non-index.md', 'index.md']), - '

link

', + self.get_rendered_result( + content='[link](index.md)', files=['non-index.md', 'index.md'] + ), + 'link', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='[link](sub2/index.md)')) def test_relative_html_link_sub_index(self): self.assertEqual( - self.get_rendered_result(['index.md', 'sub2/index.md']), - '

link

', # No trailing / + self.get_rendered_result( + content='[link](sub2/index.md)', files=['index.md', 'sub2/index.md'] + ), + 'link', ) - @mock.patch( - 'mkdocs.structure.pages.open', mock.mock_open(read_data='[link](sub2/non-index.md)') - ) def test_relative_html_link_sub_page(self): self.assertEqual( - self.get_rendered_result(['index.md', 'sub2/non-index.md']), - '

link

', # No trailing / + self.get_rendered_result( + content='[link](sub2/non-index.md)', files=['index.md', 'sub2/non-index.md'] + ), + 'link', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='[link](file%20name.md)')) def test_relative_html_link_with_encoded_space(self): self.assertEqual( - self.get_rendered_result(['index.md', 'file name.md']), - '

link

', + self.get_rendered_result( + content='[link](file%20name.md)', files=['index.md', 'file name.md'] + ), + 'link', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='[link](file name.md)')) def test_relative_html_link_with_unencoded_space(self): self.assertEqual( - self.get_rendered_result(['index.md', 'file name.md']), - '

link

', + self.get_rendered_result( + content='[link](file name.md)', files=['index.md', 'file name.md'] + ), + 'link', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='[link](../index.md)')) def test_relative_html_link_parent_index(self): self.assertEqual( - self.get_rendered_result(['sub2/non-index.md', 'index.md']), - '

link

', + self.get_rendered_result( + content='[link](../index.md)', files=['sub2/non-index.md', 'index.md'] + ), + 'link', ) - @mock.patch( - 'mkdocs.structure.pages.open', mock.mock_open(read_data='[link](non-index.md#hash)') - ) def test_relative_html_link_hash(self): self.assertEqual( - self.get_rendered_result(['index.md', 'non-index.md']), - '

link

', + self.get_rendered_result( + content='[link](non-index.md#hash)', files=['index.md', 'non-index.md'] + ), + 'link', ) - @mock.patch( - 'mkdocs.structure.pages.open', mock.mock_open(read_data='[link](sub2/index.md#hash)') - ) def test_relative_html_link_sub_index_hash(self): self.assertEqual( - self.get_rendered_result(['index.md', 'sub2/index.md']), - '

link

', + self.get_rendered_result( + content='[link](sub2/index.md#hash)', files=['index.md', 'sub2/index.md'] + ), + 'link', ) - @mock.patch( - 'mkdocs.structure.pages.open', mock.mock_open(read_data='[link](sub2/non-index.md#hash)') - ) def test_relative_html_link_sub_page_hash(self): self.assertEqual( - self.get_rendered_result(['index.md', 'sub2/non-index.md']), - '

link

', + self.get_rendered_result( + content='[link](sub2/non-index.md#hash)', files=['index.md', 'sub2/non-index.md'] + ), + 'link', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='[link](#hash)')) def test_relative_html_link_hash_only(self): self.assertEqual( - self.get_rendered_result(['index.md']), - '

link

', + self.get_rendered_result(content='[link](#hash)', files=['index.md']), + 'link', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='![image](image.png)')) def test_relative_image_link_from_homepage(self): self.assertEqual( - self.get_rendered_result(['index.md', 'image.png']), - '

image

', # no opening ./ + self.get_rendered_result( + content='![image](image.png)', files=['index.md', 'image.png'] + ), + 'image', # no opening ./ ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='![image](../image.png)')) def test_relative_image_link_from_subpage(self): self.assertEqual( - self.get_rendered_result(['sub2/non-index.md', 'image.png']), - '

image

', + self.get_rendered_result( + content='![image](../image.png)', files=['sub2/non-index.md', 'image.png'] + ), + 'image', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='![image](image.png)')) def test_relative_image_link_from_sibling(self): self.assertEqual( - self.get_rendered_result(['non-index.md', 'image.png']), - '

image

', + self.get_rendered_result( + content='![image](image.png)', files=['non-index.md', 'image.png'] + ), + 'image', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='*__not__ a link*.')) def test_no_links(self): self.assertEqual( - self.get_rendered_result(['index.md']), - '

not a link.

', + self.get_rendered_result(content='*__not__ a link*.', files=['index.md']), + 'not a link.', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='[link](non-existent.md)')) def test_bad_relative_html_link(self): - with self.assertLogs('mkdocs') as cm: - self.assertEqual( - self.get_rendered_result(['index.md']), - '

link

', - ) self.assertEqual( - '\n'.join(cm.output), - "WARNING:mkdocs.structure.pages:Documentation file 'index.md' contains a link " - "to 'non-existent.md' which is not found in the documentation files.", + self.get_rendered_result( + content='[link](non-existent.md)', + files=['index.md'], + logs="WARNING:Documentation file 'index.md' contains a link to 'non-existent.md' which is not found in the documentation files.", + ), + 'link', ) - @mock.patch( - 'mkdocs.structure.pages.open', - mock.mock_open(read_data='[external](http://example.com/index.md)'), - ) def test_external_link(self): self.assertEqual( - self.get_rendered_result(['index.md']), - '

external

', + self.get_rendered_result( + content='[external](http://example.com/index.md)', files=['index.md'] + ), + 'external', ) - @mock.patch( - 'mkdocs.structure.pages.open', mock.mock_open(read_data='[absolute link](/path/to/file.md)') - ) def test_absolute_link(self): self.assertEqual( - self.get_rendered_result(['index.md']), - '

absolute link

', + self.get_rendered_result( + content='[absolute link](/path/to/file.md)', files=['index.md'] + ), + 'absolute link', ) - @mock.patch( - 'mkdocs.structure.pages.open', - mock.mock_open(read_data='[absolute local path](\\image.png)'), - ) def test_absolute_win_local_path(self): self.assertEqual( - self.get_rendered_result(['index.md']), - '

absolute local path

', + self.get_rendered_result( + content='[absolute local path](\\image.png)', files=['index.md'] + ), + 'absolute local path', ) - @mock.patch('mkdocs.structure.pages.open', mock.mock_open(read_data='')) def test_email_link(self): self.assertEqual( - self.get_rendered_result(['index.md']), + self.get_rendered_result(content='', files=['index.md']), # Markdown's default behavior is to obscure email addresses by entity-encoding them. - # The following is equivalent to: '

mail@example.com

' - '

mail@example.com' + 'mail@' - 'example.com

', + 'example.com', ) From 4150d2b8cecccc2b971753c4e61574a4a8e9097d Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 30 Jun 2023 16:42:42 +0200 Subject: [PATCH 4/7] Configurable diagnostics around broken links to docs Expand the situations where logs about broken links are produced, make the logging levels fully configurable --- docs/user-guide/configuration.md | 74 +++++++++++- mkdocs/config/config_options.py | 17 +++ mkdocs/config/defaults.py | 35 +++++- mkdocs/structure/nav.py | 22 ++-- mkdocs/structure/pages.py | 58 ++++++---- mkdocs/tests/build_tests.py | 6 +- mkdocs/tests/structure/nav_tests.py | 2 +- mkdocs/tests/structure/page_tests.py | 162 +++++++++++++++++++++++---- 8 files changed, 311 insertions(+), 65 deletions(-) diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index bfb025ea..b433e851 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -343,17 +343,85 @@ Example: ```yaml nav: - - Foo: foo.md - - Bar: bar.md + - Foo: foo.md + - Bar: bar.md not_in_nav: | - /private.md + /private.md ``` As the previous option, this follows the .gitignore pattern format. NOTE: Adding a given file to [`exclude_docs`](#exclude_docs) takes precedence over and implies `not_in_nav`. +### validation + +NEW: **New in version 1.5.** + +Configure the strictness of MkDocs' diagnostic messages when validating links to documents. + +This is a tree of configs, and for each one the value can be one of the three: `warn`, `info`, `ignore`. Which cause a logging message of the corresponding severity to be produced. The `warn` level is, of course, intended for use with `mkdocs build --strict` (where it becomes an error), which you can employ in continuous testing. + +> EXAMPLE: **Defaults of this config as of MkDocs 1.5:** +> +> ```yaml +> validation: +> nav: +> omitted_files: info +> not_found: warn +> absolute_links: info +> links: +> not_found: warn +> absolute_links: info +> unrecognized_links: info +> ``` +> +> (Note: you shouldn't copy this whole example, because it only duplicates the defaults. Only individual items that differ should be set.) + +The defaults of some of the behaviors already differ from MkDocs 1.4 and below - they were ignored before. + +>? EXAMPLE: **Configure MkDocs 1.5 to behave like MkDocs 1.4 and below (reduce strictness):** +> +> ```yaml +> validation: +> nav: +> absolute_links: ignore +> links: +> absolute_links: ignore +> unrecognized_links: ignore +> ``` + +>! EXAMPLE: **Recommended settings for most sites (maximal strictness):** +> +> ```yaml +> validation: +> nav: +> omitted_files: warn +> absolute_links: warn +> links: +> absolute_links: warn +> unrecognized_links: warn +> ``` + +Full list of values and examples of log messages that they can hide or make more prominent: + +* `validation.nav.omitted_files` + * "The following pages exist in the docs directory, but are not included in the "nav" configuration: ..." +* `validation.nav.not_found` + * "A relative path to 'foo/bar.md' is included in the 'nav' configuration, which is not found in the documentation files." + * "A reference to 'foo/bar.md' is included in the 'nav' configuration, but this file is excluded from the built site." +* `validation.nav.absolute_links` + * "An absolute path to '/foo/bar.html' is included in the 'nav' configuration, which presumably points to an external resource." + +* `validation.links.not_found` + * "Doc file 'example.md' contains a relative link '../foo/bar.md', but the target is not found among documentation files." + * "Doc file 'example.md' contains a link to 'foo/bar.md' which is excluded from the built site." +* `validation.links.absolute_links` + * "Doc file 'example.md' contains an absolute link '/foo/bar.html', it was left as is. Did you mean 'foo/bar.md'?" +* `validation.links.unrecognized_links` + * "Doc file 'example.md' contains an unrecognized relative link '../foo/bar/', it was left as is. Did you mean 'foo/bar.md'?" + * "Doc file 'example.md' contains an unrecognized relative link 'mail\@example.com', it was left as is. Did you mean 'mailto:mail\@example.com'?" + ## Build directories ### theme diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 05994b03..9acda32e 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -2,6 +2,7 @@ from __future__ import annotations import functools import ipaddress +import logging import os import string import sys @@ -1170,3 +1171,19 @@ class PathSpec(BaseConfigOption[pathspec.gitignore.GitIgnoreSpec]): return pathspec.gitignore.GitIgnoreSpec.from_lines(lines=value.splitlines()) except ValueError as e: raise ValidationError(str(e)) + + +class _LogLevel(OptionallyRequired[int]): + levels: Mapping[str, int] = { + "warn": logging.WARNING, + "info": logging.INFO, + "ignore": logging.DEBUG, + } + + def run_validation(self, value: object) -> int: + if not isinstance(value, str): + raise ValidationError(f'Expected a string, but a {type(value)} was given.') + try: + return self.levels[value] + except KeyError: + raise ValidationError(f'Expected one of {list(self.levels)}, got {value!r}') diff --git a/mkdocs/config/defaults.py b/mkdocs/config/defaults.py index cf6e630c..c8152956 100644 --- a/mkdocs/config/defaults.py +++ b/mkdocs/config/defaults.py @@ -28,7 +28,11 @@ class MkDocsConfig(base.Config): """Gitignore-like patterns of files (relative to docs dir) to exclude from the site.""" not_in_nav = c.Optional(c.PathSpec()) - """Gitignore-like patterns of files (relative to docs dir) that are not intended to be in the nav.""" + """Gitignore-like patterns of files (relative to docs dir) that are not intended to be in the nav. + + This marks doc files that are expected not to be in the nav, otherwise they will cause a log message + (see also `validation.nav.omitted_files`). + """ site_url = c.Optional(c.URL(is_dir=True)) """The full URL to where the documentation will be hosted.""" @@ -135,6 +139,35 @@ class MkDocsConfig(base.Config): watch = c.ListOfPaths(default=[]) """A list of extra paths to watch while running `mkdocs serve`.""" + class Validation(base.Config): + class NavValidation(base.Config): + omitted_files = c._LogLevel(default='info') + """Warning level for when a doc file is never mentioned in the navigation. + For granular configuration, see `not_in_nav`.""" + + not_found = c._LogLevel(default='warn') + """Warning level for when the navigation links to a relative path that isn't an existing page on the site.""" + + absolute_links = c._LogLevel(default='info') + """Warning level for when the navigation links to an absolute path (starting with `/`).""" + + nav = c.SubConfig(NavValidation) + + class LinksValidation(base.Config): + not_found = c._LogLevel(default='warn') + """Warning level for when a Markdown doc links to a relative path that isn't an existing document on the site.""" + + absolute_links = c._LogLevel(default='info') + """Warning level for when a Markdown doc links to an absolute path (starting with `/`).""" + + unrecognized_links = c._LogLevel(default='info') + """Warning level for when a Markdown doc links to a relative path that doesn't look like + it could be a valid internal link. For example, if the link ends with `/`.""" + + links = c.SubConfig(LinksValidation) + + validation = c.SubConfig(Validation) + _current_page: mkdocs.structure.pages.Page | None = None """The currently rendered page. Please do not access this and instead rely on the `page` argument to event handlers.""" diff --git a/mkdocs/structure/nav.py b/mkdocs/structure/nav.py index 18937cf0..52eacbe6 100644 --- a/mkdocs/structure/nav.py +++ b/mkdocs/structure/nav.py @@ -148,9 +148,10 @@ def get_navigation(files: Files, config: MkDocsConfig) -> Navigation: if file.inclusion.is_in_nav(): missing_from_config.append(file.src_path) if missing_from_config: - log.info( + log.log( + config.validation.nav.omitted_files, 'The following pages exist in the docs directory, but are not ' - 'included in the "nav" configuration:\n - ' + '\n - '.join(missing_from_config) + 'included in the "nav" configuration:\n - ' + '\n - '.join(missing_from_config), ) links = _get_by_type(items, Link) @@ -159,14 +160,16 @@ def get_navigation(files: Files, config: MkDocsConfig) -> Navigation: if scheme or netloc: log.debug(f"An external link to '{link.url}' is included in the 'nav' configuration.") elif link.url.startswith('/'): - log.debug( + log.log( + config.validation.nav.absolute_links, f"An absolute path to '{link.url}' is included in the 'nav' " - "configuration, which presumably points to an external resource." + "configuration, which presumably points to an external resource.", ) else: - log.warning( + log.log( + config.validation.nav.not_found, f"A relative path to '{link.url}' is included in the 'nav' " - "configuration, which is not found in the documentation files." + "configuration, which is not found in the documentation files.", ) return Navigation(items, pages) @@ -190,9 +193,10 @@ def _data_to_navigation(data, files: Files, config: MkDocsConfig): file = files.get_file_from_path(path) if file: if file.inclusion.is_excluded(): - log.info( - f"A relative path to '{file.src_path}' is included in the 'nav' " - "configuration, but this file is excluded from the built site." + log.log( + min(logging.INFO, config.validation.nav.not_found), + f"A reference to '{file.src_path}' is included in the 'nav' " + "configuration, but this file is excluded from the built site.", ) return Page(title, file, config) return Link(title, path) diff --git a/mkdocs/structure/pages.py b/mkdocs/structure/pages.py index 4ee36aca..43e2b482 100644 --- a/mkdocs/structure/pages.py +++ b/mkdocs/structure/pages.py @@ -2,7 +2,6 @@ from __future__ import annotations import copy import logging -import os import posixpath import warnings from typing import TYPE_CHECKING, Any, Callable, MutableMapping @@ -268,7 +267,7 @@ class Page(StructureItem): extension_configs=config['mdx_configs'] or {}, ) - relative_path_ext = _RelativePathTreeprocessor(self.file, files) + relative_path_ext = _RelativePathTreeprocessor(self.file, files, config) relative_path_ext._register(md) extract_title_ext = _ExtractTitleTreeprocessor() @@ -280,9 +279,10 @@ class Page(StructureItem): class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): - def __init__(self, file: File, files: Files) -> None: + def __init__(self, file: File, files: Files, config: MkDocsConfig) -> None: self.file = file self.files = files + self.config = config def run(self, root: etree.Element) -> etree.Element: """ @@ -309,18 +309,18 @@ class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): def path_to_url(self, url: str) -> str: scheme, netloc, path, query, fragment = urlsplit(url) - if ( - scheme - or netloc - or not path - or url.startswith('/') - or url.startswith('\\') - or AMP_SUBSTITUTE in url - or '.' not in os.path.split(path)[-1] - ): - # Ignore URLs unless they are a relative link to a source file. - # AMP_SUBSTITUTE is used internally by Markdown only for email. - # No '.' in the last part of a path indicates path does not point to a file. + # Ignore URLs unless they are a relative link to a source file. + if scheme or netloc: # External link. + return url + elif url.startswith('/') or url.startswith('\\'): # Absolute link. + log.log( + self.config.validation.links.absolute_links, + f"Doc file '{self.file.src_uri}' contains an absolute link '{url}', it was left as is.", + ) + return url + elif AMP_SUBSTITUTE in url: # AMP_SUBSTITUTE is used internally by Markdown only for email. + return url + elif not path: # Self-link containing only query or fragment. return url # Determine the filepath of the target. @@ -330,19 +330,29 @@ class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): # Validate that the target exists in files collection. target_file = self.files.get_file_from_path(target_uri) if target_file is None: - log.warning( - f"Documentation file '{self.file.src_uri}' contains a link to " - f"'{target_uri}' which is not found in the documentation files." - ) + if '.' not in posixpath.split(path)[-1]: + # No '.' in the last part of a path indicates path does not point to a file. + log.log( + self.config.validation.links.unrecognized_links, + f"Doc file '{self.file.src_uri}' contains an unrecognized relative link '{url}', " + f"it was left as is.", + ) + else: + target = f" '{target_uri}'" if target_uri != url else "" + log.log( + self.config.validation.links.not_found, + f"Doc file '{self.file.src_uri}' contains a relative link '{url}', " + f"but the target{target} is not found among documentation files.", + ) return url if target_file.inclusion.is_excluded(): - log.info( - f"Documentation file '{self.file.src_uri}' contains a link to " - f"'{target_uri}' which is excluded from the built site." + log.log( + min(logging.INFO, self.config.validation.links.not_found), + f"Doc file '{self.file.src_uri}' contains a link to " + f"'{target_uri}' which is excluded from the built site.", ) path = target_file.url_relative_to(self.file) - components = (scheme, netloc, path, query, fragment) - return urlunsplit(components) + return urlunsplit(('', '', path, query, fragment)) def _register(self, md: markdown.Markdown) -> None: md.treeprocessors.register(self, "relpath", 0) diff --git a/mkdocs/tests/build_tests.py b/mkdocs/tests/build_tests.py index 8d0eb0fa..3ea64d42 100644 --- a/mkdocs/tests/build_tests.py +++ b/mkdocs/tests/build_tests.py @@ -599,7 +599,7 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): with self.subTest(live_server=None): expected_logs = ''' - INFO:Documentation file 'test/foo.md' contains a link to 'test/bar.md' which is excluded from the built site. + INFO:Doc file 'test/foo.md' contains a link to 'test/bar.md' which is excluded from the built site. ''' with self._assert_build_logs(expected_logs): build.build(cfg) @@ -610,8 +610,8 @@ class BuildTests(PathAssertionMixin, unittest.TestCase): server = testing_server(site_dir, mount_path='/documentation/') with self.subTest(live_server=server): expected_logs = ''' - INFO:Documentation file 'test/bar.md' contains a link to 'test/baz.md' which is excluded from the built site. - INFO:Documentation file 'test/foo.md' contains a link to 'test/bar.md' which is excluded from the built site. + INFO:Doc file 'test/bar.md' contains a link to 'test/baz.md' which is excluded from the built site. + INFO:Doc file 'test/foo.md' contains a link to 'test/bar.md' which is excluded from the built site. INFO:The following pages are being built only for the preview but will be excluded from `mkdocs build` per `exclude_docs`: - http://localhost:123/documentation/.zoo.html - http://localhost:123/documentation/test/bar.html diff --git a/mkdocs/tests/structure/nav_tests.py b/mkdocs/tests/structure/nav_tests.py index 1ef971f6..3e4aba5f 100644 --- a/mkdocs/tests/structure/nav_tests.py +++ b/mkdocs/tests/structure/nav_tests.py @@ -123,7 +123,7 @@ class SiteNavigationTests(unittest.TestCase): self.assertEqual( cm.output, [ - "DEBUG:mkdocs.structure.nav:An absolute path to '/local.html' is included in the 'nav' configuration, which presumably points to an external resource.", + "INFO:mkdocs.structure.nav:An absolute path to '/local.html' is included in the 'nav' configuration, which presumably points to an external resource.", "DEBUG:mkdocs.structure.nav:An external link to 'http://example.com/external.html' is included in the 'nav' configuration.", ], ) diff --git a/mkdocs/tests/structure/page_tests.py b/mkdocs/tests/structure/page_tests.py index 7d5160f3..e4dab0bd 100644 --- a/mkdocs/tests/structure/page_tests.py +++ b/mkdocs/tests/structure/page_tests.py @@ -1,6 +1,5 @@ from __future__ import annotations -import functools import os import sys import textwrap @@ -10,11 +9,26 @@ from unittest import mock from mkdocs.config.defaults import MkDocsConfig from mkdocs.structure.files import File, Files from mkdocs.structure.pages import Page -from mkdocs.tests.base import dedent, load_config, tempdir +from mkdocs.tests.base import dedent, tempdir -load_config = functools.lru_cache(maxsize=None)(load_config) +DOCS_DIR = os.path.join( + os.path.abspath(os.path.dirname(__file__)), '..', 'integration', 'subpages', 'docs' +) -DOCS_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), '../integration/subpages/docs') + +def load_config(**cfg) -> MkDocsConfig: + cfg.setdefault('site_name', 'Example') + cfg.setdefault( + 'docs_dir', + os.path.join( + os.path.abspath(os.path.dirname(__file__)), '..', 'integration', 'minimal', 'docs' + ), + ) + conf = MkDocsConfig() + conf.load_dict(cfg) + errors_warnings = conf.validate() + assert errors_warnings == ([], []), errors_warnings + return conf class PageTests(unittest.TestCase): @@ -748,6 +762,14 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'link', ) + self.assertEqual( + self.get_rendered_result( + use_directory_urls=False, + content='[link](non-index.md)', + files=['index.md', 'non-index.md'], + ), + 'link', + ) def test_relative_html_link_index(self): self.assertEqual( @@ -756,6 +778,14 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'link', ) + self.assertEqual( + self.get_rendered_result( + use_directory_urls=False, + content='[link](index.md)', + files=['non-index.md', 'index.md'], + ), + 'link', + ) def test_relative_html_link_sub_index(self): self.assertEqual( @@ -764,6 +794,14 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'link', ) + self.assertEqual( + self.get_rendered_result( + use_directory_urls=False, + content='[link](sub2/index.md)', + files=['index.md', 'sub2/index.md'], + ), + 'link', + ) def test_relative_html_link_sub_page(self): self.assertEqual( @@ -772,6 +810,14 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'link', ) + self.assertEqual( + self.get_rendered_result( + use_directory_urls=False, + content='[link](sub2/non-index.md)', + files=['index.md', 'sub2/non-index.md'], + ), + 'link', + ) def test_relative_html_link_with_encoded_space(self): self.assertEqual( @@ -784,9 +830,11 @@ class RelativePathExtensionTests(unittest.TestCase): def test_relative_html_link_with_unencoded_space(self): self.assertEqual( self.get_rendered_result( - content='[link](file name.md)', files=['index.md', 'file name.md'] + use_directory_urls=False, + content='[link](file name.md)', + files=['index.md', 'file name.md'], ), - 'link', + 'link', ) def test_relative_html_link_parent_index(self): @@ -796,6 +844,14 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'link', ) + self.assertEqual( + self.get_rendered_result( + use_directory_urls=False, + content='[link](../index.md)', + files=['sub2/non-index.md', 'index.md'], + ), + 'link', + ) def test_relative_html_link_hash(self): self.assertEqual( @@ -812,6 +868,14 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'link', ) + self.assertEqual( + self.get_rendered_result( + use_directory_urls=False, + content='[link](sub2/index.md#hash)', + files=['index.md', 'sub2/index.md'], + ), + 'link', + ) def test_relative_html_link_sub_page_hash(self): self.assertEqual( @@ -822,18 +886,26 @@ class RelativePathExtensionTests(unittest.TestCase): ) def test_relative_html_link_hash_only(self): - self.assertEqual( - self.get_rendered_result(content='[link](#hash)', files=['index.md']), - 'link', - ) + for use_directory_urls in True, False: + self.assertEqual( + self.get_rendered_result( + use_directory_urls=use_directory_urls, + content='[link](#hash)', + files=['index.md'], + ), + 'link', + ) def test_relative_image_link_from_homepage(self): - self.assertEqual( - self.get_rendered_result( - content='![image](image.png)', files=['index.md', 'image.png'] - ), - 'image', # no opening ./ - ) + for use_directory_urls in True, False: + self.assertEqual( + self.get_rendered_result( + use_directory_urls=use_directory_urls, + content='![image](image.png)', + files=['index.md', 'image.png'], + ), + 'image', # no opening ./ + ) def test_relative_image_link_from_subpage(self): self.assertEqual( @@ -850,6 +922,14 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'image', ) + self.assertEqual( + self.get_rendered_result( + use_directory_urls=False, + content='![image](image.png)', + files=['non-index.md', 'image.png'], + ), + 'image', + ) def test_no_links(self): self.assertEqual( @@ -862,10 +942,19 @@ class RelativePathExtensionTests(unittest.TestCase): self.get_rendered_result( content='[link](non-existent.md)', files=['index.md'], - logs="WARNING:Documentation file 'index.md' contains a link to 'non-existent.md' which is not found in the documentation files.", + logs="WARNING:Doc file 'index.md' contains a relative link 'non-existent.md', but the target is not found among documentation files.", ), 'link', ) + self.assertEqual( + self.get_rendered_result( + validation=dict(links=dict(not_found='info')), + content='[link](../non-existent.md)', + files=['sub/index.md'], + logs="INFO:Doc file 'sub/index.md' contains a relative link '../non-existent.md', but the target 'non-existent.md' is not found among documentation files.", + ), + 'link', + ) def test_external_link(self): self.assertEqual( @@ -878,18 +967,33 @@ class RelativePathExtensionTests(unittest.TestCase): def test_absolute_link(self): self.assertEqual( self.get_rendered_result( - content='[absolute link](/path/to/file.md)', files=['index.md'] + content='[absolute link](/path/to/file.md)', + files=['index.md'], + logs="INFO:Doc file 'index.md' contains an absolute link '/path/to/file.md', it was left as is.", + ), + 'absolute link', + ) + self.assertEqual( + self.get_rendered_result( + validation=dict(links=dict(absolute_links='warn')), + content='[absolute link](/path/to/file.md)', + files=['index.md'], + logs="WARNING:Doc file 'index.md' contains an absolute link '/path/to/file.md', it was left as is.", ), 'absolute link', ) def test_absolute_win_local_path(self): - self.assertEqual( - self.get_rendered_result( - content='[absolute local path](\\image.png)', files=['index.md'] - ), - 'absolute local path', - ) + for use_directory_urls in True, False: + self.assertEqual( + self.get_rendered_result( + use_directory_urls=use_directory_urls, + content='[absolute local path](\\image.png)', + files=['index.md'], + logs="INFO:Doc file 'index.md' contains an absolute link '\\image.png', it was left as is.", + ), + 'absolute local path', + ) def test_email_link(self): self.assertEqual( @@ -900,3 +1004,13 @@ class RelativePathExtensionTests(unittest.TestCase): 'xample.com">mail@' 'example.com', ) + + def test_invalid_email_link(self): + self.assertEqual( + self.get_rendered_result( + content='[contact](mail@example.com)', + files=['index.md'], + logs="WARNING:Doc file 'index.md' contains a relative link 'mail@example.com', but the target is not found among documentation files.", + ), + 'contact', + ) From ca5160af27c1948b7dfa716127ed70a438622371 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 1 Jul 2023 15:24:17 +0200 Subject: [PATCH 5/7] Add suggestions for malformed links in Markdown Guess what existing page might have been meant based on the provided link, and mention it in the log message. --- mkdocs/structure/pages.py | 102 +++++++++++++++++++++------ mkdocs/tests/structure/page_tests.py | 35 +++++++-- 2 files changed, 111 insertions(+), 26 deletions(-) diff --git a/mkdocs/structure/pages.py b/mkdocs/structure/pages.py index 43e2b482..3b71eef3 100644 --- a/mkdocs/structure/pages.py +++ b/mkdocs/structure/pages.py @@ -4,7 +4,7 @@ import copy import logging import posixpath import warnings -from typing import TYPE_CHECKING, Any, Callable, MutableMapping +from typing import TYPE_CHECKING, Any, Callable, Iterator, MutableMapping from urllib.parse import unquote as urlunquote from urllib.parse import urljoin, urlsplit, urlunsplit @@ -14,6 +14,7 @@ import markdown.postprocessors import markdown.treeprocessors from markdown.util import AMP_SUBSTITUTE +from mkdocs import utils from mkdocs.structure import StructureItem from mkdocs.structure.toc import get_toc from mkdocs.utils import get_build_date, get_markdown_title, meta, weak_property @@ -306,52 +307,109 @@ class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): return root + @classmethod + def _target_uri(cls, src_path: str, dest_path: str): + return posixpath.normpath(posixpath.join(src_path, dest_path).lstrip('/')) + + @classmethod + def _target_uris(cls, use_directory_urls: bool, file: File, path: str) -> Iterator[str]: + """First yields the resolved file uri for the link, then proceeds to yield guesses for possible mistakes.""" + target_uri = cls._target_uri(posixpath.dirname(file.src_uri), path) + yield target_uri + + tried = {target_uri} + prefixes = [target_uri] + if use_directory_urls and file.name != 'index': + # User might have added an extra '../' because that's how to make an invalid link work with use_directory_urls. + prefixes.insert(0, cls._target_uri(file.src_uri, path)) + suffixes = ['/index.md', '/README.md'] + if not target_uri.endswith('/') and '.' in posixpath.basename(path): + suffixes.insert(0, '') + if use_directory_urls and path and '.' not in posixpath.basename(path): + suffixes.insert(0, '.md') + + for pref in prefixes: + for suf in suffixes: + if pref == '.' and not suf.startswith('/'): + continue + guess = posixpath.normpath(pref + suf) + if guess not in tried: + yield guess + tried.add(guess) + def path_to_url(self, url: str) -> str: scheme, netloc, path, query, fragment = urlsplit(url) + warning_level, warning = 0, '' + # Ignore URLs unless they are a relative link to a source file. if scheme or netloc: # External link. return url elif url.startswith('/') or url.startswith('\\'): # Absolute link. - log.log( - self.config.validation.links.absolute_links, - f"Doc file '{self.file.src_uri}' contains an absolute link '{url}', it was left as is.", - ) - return url + warning_level = self.config.validation.links.absolute_links + warning = f"Doc file '{self.file.src_uri}' contains an absolute link '{url}', it was left as is." elif AMP_SUBSTITUTE in url: # AMP_SUBSTITUTE is used internally by Markdown only for email. return url elif not path: # Self-link containing only query or fragment. return url + path = urlunquote(path) # Determine the filepath of the target. - target_uri = posixpath.join(posixpath.dirname(self.file.src_uri), urlunquote(path)) - target_uri = posixpath.normpath(target_uri).lstrip('/') + possible_target_uris = self._target_uris(self.config.use_directory_urls, self.file, path) - # Validate that the target exists in files collection. - target_file = self.files.get_file_from_path(target_uri) - if target_file is None: - if '.' not in posixpath.split(path)[-1]: + if warning: + # For absolute path (already has a warning), the primary lookup path should be preserved as a tip option. + target_uri = url + target_file = None + else: + # Validate that the target exists in files collection. + target_uri = next(possible_target_uris) + target_file = self.files.get_file_from_path(target_uri) + + if target_file is None and not warning: + # Primary lookup path had no match, definitely produce a warning, just choose which one. + if '.' not in posixpath.basename(path): # No '.' in the last part of a path indicates path does not point to a file. - log.log( - self.config.validation.links.unrecognized_links, + warning_level = self.config.validation.links.unrecognized_links + warning = ( f"Doc file '{self.file.src_uri}' contains an unrecognized relative link '{url}', " - f"it was left as is.", + f"it was left as is." ) else: target = f" '{target_uri}'" if target_uri != url else "" - log.log( - self.config.validation.links.not_found, + warning_level = self.config.validation.links.not_found + warning = ( f"Doc file '{self.file.src_uri}' contains a relative link '{url}', " - f"but the target{target} is not found among documentation files.", + f"but the target{target} is not found among documentation files." ) + + if warning: + # There was no match, so try to guess what other file could've been intended. + if warning_level > logging.DEBUG: + suggest_url = '' + for path in possible_target_uris: + if self.files.get_file_from_path(path) is not None: + path = utils.get_relative_url(path, self.file.url) + suggest_url = urlunsplit(('', '', path, query, fragment)) + break + else: + if '@' in url and '.' in url and '/' not in url: + suggest_url = f'mailto:{url}' + if suggest_url: + warning += f" Did you mean '{suggest_url}'?" + log.log(warning_level, warning) return url + + assert target_uri is not None + assert target_file is not None if target_file.inclusion.is_excluded(): - log.log( - min(logging.INFO, self.config.validation.links.not_found), + warning_level = min(logging.INFO, self.config.validation.links.not_found) + warning = ( f"Doc file '{self.file.src_uri}' contains a link to " - f"'{target_uri}' which is excluded from the built site.", + f"'{target_uri}' which is excluded from the built site." ) - path = target_file.url_relative_to(self.file) + log.log(warning_level, warning) + path = utils.get_relative_url(target_file.url, self.file.url) return urlunsplit(('', '', path, query, fragment)) def _register(self, md: markdown.Markdown) -> None: diff --git a/mkdocs/tests/structure/page_tests.py b/mkdocs/tests/structure/page_tests.py index e4dab0bd..db38a324 100644 --- a/mkdocs/tests/structure/page_tests.py +++ b/mkdocs/tests/structure/page_tests.py @@ -964,15 +964,34 @@ class RelativePathExtensionTests(unittest.TestCase): 'external', ) - def test_absolute_link(self): + def test_absolute_link_with_suggestion(self): self.assertEqual( self.get_rendered_result( content='[absolute link](/path/to/file.md)', - files=['index.md'], - logs="INFO:Doc file 'index.md' contains an absolute link '/path/to/file.md', it was left as is.", + files=['index.md', 'path/to/file.md'], + logs="INFO:Doc file 'index.md' contains an absolute link '/path/to/file.md', it was left as is. Did you mean 'path/to/file.md'?", ), 'absolute link', ) + self.assertEqual( + self.get_rendered_result( + use_directory_urls=False, + content='[absolute link](/path/to/file/)', + files=['path/index.md', 'path/to/file.md'], + logs="INFO:Doc file 'path/index.md' contains an absolute link '/path/to/file/', it was left as is.", + ), + 'absolute link', + ) + self.assertEqual( + self.get_rendered_result( + content='[absolute link](/path/to/file)', + files=['path/index.md', 'path/to/file.md'], + logs="INFO:Doc file 'path/index.md' contains an absolute link '/path/to/file', it was left as is. Did you mean 'to/file.md'?", + ), + 'absolute link', + ) + + def test_absolute_link(self): self.assertEqual( self.get_rendered_result( validation=dict(links=dict(absolute_links='warn')), @@ -982,6 +1001,14 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'absolute link', ) + self.assertEqual( + self.get_rendered_result( + validation=dict(links=dict(absolute_links='ignore')), + content='[absolute link](/path/to/file.md)', + files=['index.md'], + ), + 'absolute link', + ) def test_absolute_win_local_path(self): for use_directory_urls in True, False: @@ -1010,7 +1037,7 @@ class RelativePathExtensionTests(unittest.TestCase): self.get_rendered_result( content='[contact](mail@example.com)', files=['index.md'], - logs="WARNING:Doc file 'index.md' contains a relative link 'mail@example.com', but the target is not found among documentation files.", + logs="WARNING:Doc file 'index.md' contains a relative link 'mail@example.com', but the target is not found among documentation files. Did you mean 'mailto:mail@example.com'?", ), 'contact', ) From aef55990b9c6054d25dbdfdaa9cf3a0e833add93 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 30 Jun 2023 23:26:19 +0200 Subject: [PATCH 6/7] Propagate 1st-level `validation` keys to the 2nd-level sub-dicts --- docs/user-guide/configuration.md | 18 ++--- mkdocs/config/config_options.py | 22 +++++ mkdocs/config/defaults.py | 2 +- mkdocs/tests/config/config_options_tests.py | 90 +++++++++++++++++++++ 4 files changed, 120 insertions(+), 12 deletions(-) diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index b433e851..25d0216a 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -384,25 +384,21 @@ The defaults of some of the behaviors already differ from MkDocs 1.4 and below - > > ```yaml > validation: -> nav: -> absolute_links: ignore -> links: -> absolute_links: ignore -> unrecognized_links: ignore +> absolute_links: ignore +> unrecognized_links: ignore > ``` >! EXAMPLE: **Recommended settings for most sites (maximal strictness):** > > ```yaml > validation: -> nav: -> omitted_files: warn -> absolute_links: warn -> links: -> absolute_links: warn -> unrecognized_links: warn +> omitted_files: warn +> absolute_links: warn +> unrecognized_links: warn > ``` +Note how in the above examples we omitted the 'nav' and 'links' keys. Here `absolute_links:` means setting both `nav: absolute_links:` and `links: absolute_links:`. + Full list of values and examples of log messages that they can hide or make more prominent: * `validation.nav.omitted_files` diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 9acda32e..09be435f 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -119,6 +119,28 @@ class SubConfig(Generic[SomeConfig], BaseConfigOption[SomeConfig]): return config +class PropagatingSubConfig(SubConfig[SomeConfig], Generic[SomeConfig]): + """A SubConfig that must consist of SubConfigs with defined schemas. + + Any value set on the top config gets moved to sub-configs with matching keys. + """ + + def run_validation(self, value: object): + if isinstance(value, dict): + to_discard = set() + for k1, v1 in self.config_class._schema: + if isinstance(v1, SubConfig): + for k2, _ in v1.config_class._schema: + if k2 in value: + subdict = value.setdefault(k1, {}) + if isinstance(subdict, dict): + to_discard.add(k2) + subdict.setdefault(k2, value[k2]) + for k in to_discard: + del value[k] + return super().run_validation(value) + + class OptionallyRequired(Generic[T], BaseConfigOption[T]): """ Soft-deprecated, do not use. diff --git a/mkdocs/config/defaults.py b/mkdocs/config/defaults.py index c8152956..3f94eb54 100644 --- a/mkdocs/config/defaults.py +++ b/mkdocs/config/defaults.py @@ -166,7 +166,7 @@ class MkDocsConfig(base.Config): links = c.SubConfig(LinksValidation) - validation = c.SubConfig(Validation) + validation = c.PropagatingSubConfig[Validation]() _current_page: mkdocs.structure.pages.Page | None = None """The currently rendered page. Please do not access this and instead diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index e13a20ca..2b9d9af4 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -3,6 +3,7 @@ from __future__ import annotations import contextlib import copy import io +import logging import os import re import sys @@ -21,6 +22,7 @@ else: import mkdocs from mkdocs.config import config_options as c +from mkdocs.config import defaults from mkdocs.config.base import Config from mkdocs.plugins import BasePlugin, PluginCollection from mkdocs.tests.base import tempdir @@ -1573,6 +1575,94 @@ class SubConfigTest(TestCase): self.assertEqual(passed_config_path, config_path) +class NestedSubConfigTest(TestCase): + def defaults(self): + return { + 'nav': { + 'omitted_files': logging.INFO, + 'not_found': logging.WARNING, + 'absolute_links': logging.INFO, + }, + 'links': { + 'not_found': logging.WARNING, + 'absolute_links': logging.INFO, + 'unrecognized_links': logging.INFO, + }, + } + + class Schema(Config): + validation = c.PropagatingSubConfig[defaults.MkDocsConfig.Validation]() + + def test_unspecified(self) -> None: + for cfg in {}, {'validation': {}}: + with self.subTest(cfg): + conf = self.get_config( + self.Schema, + {}, + ) + self.assertEqual(conf.validation, self.defaults()) + + def test_sets_nested_and_not_nested(self) -> None: + conf = self.get_config( + self.Schema, + {'validation': {'not_found': 'ignore', 'links': {'absolute_links': 'warn'}}}, + ) + expected = self.defaults() + expected['nav']['not_found'] = logging.DEBUG + expected['links']['not_found'] = logging.DEBUG + expected['links']['absolute_links'] = logging.WARNING + self.assertEqual(conf.validation, expected) + + def test_sets_nested_different(self) -> None: + conf = self.get_config( + self.Schema, + {'validation': {'not_found': 'ignore', 'links': {'not_found': 'warn'}}}, + ) + expected = self.defaults() + expected['nav']['not_found'] = logging.DEBUG + expected['links']['not_found'] = logging.WARNING + self.assertEqual(conf.validation, expected) + + def test_sets_only_one_nested(self) -> None: + conf = self.get_config( + self.Schema, + {'validation': {'omitted_files': 'ignore'}}, + ) + expected = self.defaults() + expected['nav']['omitted_files'] = logging.DEBUG + self.assertEqual(conf.validation, expected) + + def test_sets_nested_not_dict(self) -> None: + with self.expect_error( + validation="Sub-option 'links': Sub-option 'unrecognized_links': Expected a string, but a was given." + ): + self.get_config( + self.Schema, + {'validation': {'unrecognized_links': [], 'links': {'absolute_links': 'warn'}}}, + ) + + def test_wrong_key_nested(self) -> None: + conf = self.get_config( + self.Schema, + {'validation': {'foo': 'warn', 'not_found': 'warn'}}, + warnings=dict(validation="Sub-option 'foo': Unrecognised configuration name: foo"), + ) + expected = self.defaults() + expected['nav']['not_found'] = logging.WARNING + expected['links']['not_found'] = logging.WARNING + expected['foo'] = 'warn' + self.assertEqual(conf.validation, expected) + + def test_wrong_type_nested(self) -> None: + with self.expect_error( + validation="Sub-option 'nav': Sub-option 'omitted_files': Expected one of ['warn', 'info', 'ignore'], got 'hi'" + ): + self.get_config( + self.Schema, + {'validation': {'omitted_files': 'hi'}}, + ) + + class MarkdownExtensionsTest(TestCase): @patch('markdown.Markdown') def test_simple_list(self, mock_md) -> None: From ba6d23e7217e3cee05897f30a411dec8dc459c73 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 8 Jul 2023 13:28:12 +0200 Subject: [PATCH 7/7] More rigorous suggestions and tests for relative links in Markdown --- mkdocs/structure/pages.py | 59 +++++--- mkdocs/tests/structure/page_tests.py | 192 ++++++++++++++++++++++++++- mkdocs/utils/__init__.py | 10 ++ 3 files changed, 239 insertions(+), 22 deletions(-) diff --git a/mkdocs/structure/pages.py b/mkdocs/structure/pages.py index 3b71eef3..7b84e1fc 100644 --- a/mkdocs/structure/pages.py +++ b/mkdocs/structure/pages.py @@ -17,7 +17,7 @@ from markdown.util import AMP_SUBSTITUTE from mkdocs import utils from mkdocs.structure import StructureItem from mkdocs.structure.toc import get_toc -from mkdocs.utils import get_build_date, get_markdown_title, meta, weak_property +from mkdocs.utils import _removesuffix, get_build_date, get_markdown_title, meta, weak_property if TYPE_CHECKING: from xml.etree import ElementTree as etree @@ -309,31 +309,45 @@ class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): @classmethod def _target_uri(cls, src_path: str, dest_path: str): - return posixpath.normpath(posixpath.join(src_path, dest_path).lstrip('/')) + return posixpath.normpath( + posixpath.join(posixpath.dirname(src_path), dest_path).lstrip('/') + ) @classmethod - def _target_uris(cls, use_directory_urls: bool, file: File, path: str) -> Iterator[str]: + def _possible_target_uris( + cls, file: File, path: str, use_directory_urls: bool + ) -> Iterator[str]: """First yields the resolved file uri for the link, then proceeds to yield guesses for possible mistakes.""" - target_uri = cls._target_uri(posixpath.dirname(file.src_uri), path) + target_uri = cls._target_uri(file.src_uri, path) yield target_uri + if posixpath.normpath(path) == '.': + # Explicitly link to current file. + yield file.src_uri + return tried = {target_uri} - prefixes = [target_uri] - if use_directory_urls and file.name != 'index': - # User might have added an extra '../' because that's how to make an invalid link work with use_directory_urls. - prefixes.insert(0, cls._target_uri(file.src_uri, path)) - suffixes = ['/index.md', '/README.md'] - if not target_uri.endswith('/') and '.' in posixpath.basename(path): - suffixes.insert(0, '') - if use_directory_urls and path and '.' not in posixpath.basename(path): - suffixes.insert(0, '.md') + + prefixes = [target_uri, cls._target_uri(file.url, path)] + if prefixes[0] == prefixes[1]: + prefixes.pop() + + suffixes: list[Callable[[str], str]] = [] + if use_directory_urls: + suffixes.append(lambda p: p) + if not posixpath.splitext(target_uri)[-1]: + suffixes.append(lambda p: posixpath.join(p, 'index.md')) + suffixes.append(lambda p: posixpath.join(p, 'README.md')) + if ( + not target_uri.endswith('.') + and not path.endswith('.md') + and (use_directory_urls or not path.endswith('/')) + ): + suffixes.append(lambda p: _removesuffix(p, '.html') + '.md') for pref in prefixes: for suf in suffixes: - if pref == '.' and not suf.startswith('/'): - continue - guess = posixpath.normpath(pref + suf) - if guess not in tried: + guess = posixpath.normpath(suf(pref)) + if guess not in tried and not guess.startswith('../'): yield guess tried.add(guess) @@ -355,7 +369,9 @@ class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): path = urlunquote(path) # Determine the filepath of the target. - possible_target_uris = self._target_uris(self.config.use_directory_urls, self.file, path) + possible_target_uris = self._possible_target_uris( + self.file, path, self.config.use_directory_urls + ) if warning: # For absolute path (already has a warning), the primary lookup path should be preserved as a tip option. @@ -368,7 +384,7 @@ class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): if target_file is None and not warning: # Primary lookup path had no match, definitely produce a warning, just choose which one. - if '.' not in posixpath.basename(path): + if not posixpath.splitext(path)[-1]: # No '.' in the last part of a path indicates path does not point to a file. warning_level = self.config.validation.links.unrecognized_links warning = ( @@ -389,7 +405,10 @@ class _RelativePathTreeprocessor(markdown.treeprocessors.Treeprocessor): suggest_url = '' for path in possible_target_uris: if self.files.get_file_from_path(path) is not None: - path = utils.get_relative_url(path, self.file.url) + if fragment and path == self.file.src_uri: + path = '' + else: + path = utils.get_relative_url(path, self.file.src_uri) suggest_url = urlunsplit(('', '', path, query, fragment)) break else: diff --git a/mkdocs/tests/structure/page_tests.py b/mkdocs/tests/structure/page_tests.py index db38a324..d196cffa 100644 --- a/mkdocs/tests/structure/page_tests.py +++ b/mkdocs/tests/structure/page_tests.py @@ -8,7 +8,7 @@ from unittest import mock from mkdocs.config.defaults import MkDocsConfig from mkdocs.structure.files import File, Files -from mkdocs.structure.pages import Page +from mkdocs.structure.pages import Page, _RelativePathTreeprocessor from mkdocs.tests.base import dedent, tempdir DOCS_DIR = os.path.join( @@ -937,7 +937,7 @@ class RelativePathExtensionTests(unittest.TestCase): 'not a link.', ) - def test_bad_relative_html_link(self): + def test_bad_relative_doc_link(self): self.assertEqual( self.get_rendered_result( content='[link](non-existent.md)', @@ -956,6 +956,35 @@ class RelativePathExtensionTests(unittest.TestCase): 'link', ) + def test_relative_slash_link_with_suggestion(self): + self.assertEqual( + self.get_rendered_result( + content='[link](../about/)', + files=['foo/index.md', 'about.md'], + logs="INFO:Doc file 'foo/index.md' contains an unrecognized relative link '../about/', it was left as is. Did you mean '../about.md'?", + ), + 'link', + ) + self.assertEqual( + self.get_rendered_result( + validation=dict(links=dict(unrecognized_links='warn')), + content='[link](../#example)', + files=['foo/bar.md', 'index.md'], + logs="WARNING:Doc file 'foo/bar.md' contains an unrecognized relative link '../#example', it was left as is. Did you mean '../index.md#example'?", + ), + 'link', + ) + + def test_self_anchor_link_with_suggestion(self): + self.assertEqual( + self.get_rendered_result( + content='[link](./#test)', + files=['index.md'], + logs="INFO:Doc file 'index.md' contains an unrecognized relative link './#test', it was left as is. Did you mean '#test'?", + ), + 'link', + ) + def test_external_link(self): self.assertEqual( self.get_rendered_result( @@ -1010,6 +1039,24 @@ class RelativePathExtensionTests(unittest.TestCase): 'absolute link', ) + def test_image_link_with_suggestion(self): + self.assertEqual( + self.get_rendered_result( + content='![image](../image.png)', + files=['foo/bar.md', 'foo/image.png'], + logs="WARNING:Doc file 'foo/bar.md' contains a relative link '../image.png', but the target 'image.png' is not found among documentation files. Did you mean 'image.png'?", + ), + 'image', + ) + self.assertEqual( + self.get_rendered_result( + content='![image](/image.png)', + files=['foo/bar.md', 'image.png'], + logs="INFO:Doc file 'foo/bar.md' contains an absolute link '/image.png', it was left as is. Did you mean '../image.png'?", + ), + 'image', + ) + def test_absolute_win_local_path(self): for use_directory_urls in True, False: self.assertEqual( @@ -1041,3 +1088,144 @@ class RelativePathExtensionTests(unittest.TestCase): ), 'contact', ) + + def test_possible_target_uris(self): + def test(paths, expected='', exp_true=None, exp_false=None): + """Test that `possible_target_uris` yields expected values, for use_directory_urls = true and false""" + for use_directory_urls, expected in ( + (True, exp_true or expected), + (False, exp_false or expected), + ): + with self.subTest(paths, use_directory_urls=use_directory_urls): + src_path, dest_path = paths + f = File(src_path, '', '', use_directory_urls) + actual = _RelativePathTreeprocessor._possible_target_uris( + f, dest_path, use_directory_urls + ) + self.assertEqual(list(actual), expected.split(', ')) + + test(('index.md', 'index.md'), expected='index.md') + test(('index.md', 'foo/bar.md'), expected='foo/bar.md') + test( + ('index.md', 'foo/bar'), + expected='foo/bar, foo/bar/index.md, foo/bar/README.md, foo/bar.md', + ) + + test(('index.md', 'foo/bar.html'), expected='foo/bar.html, foo/bar.md') + test( + ('foo.md', 'foo/bar.html'), + exp_true='foo/bar.html, foo/bar.md, foo/foo/bar.html, foo/foo/bar.md', + exp_false='foo/bar.html, foo/bar.md', + ) + + test(('foo.md', 'index.md'), exp_true='index.md, foo/index.md', exp_false='index.md') + test(('foo.md', 'foo.md'), exp_true='foo.md, foo/foo.md', exp_false='foo.md') + test(('foo.md', 'bar.md'), exp_true='bar.md, foo/bar.md', exp_false='bar.md') + test( + ('foo.md', 'foo/bar.md'), exp_true='foo/bar.md, foo/foo/bar.md', exp_false='foo/bar.md' + ) + test( + ('foo.md', 'foo'), + exp_true='foo, foo/index.md, foo/README.md, foo.md, foo/foo, foo/foo/index.md, foo/foo/README.md, foo/foo.md', + exp_false='foo, foo/index.md, foo/README.md, foo.md', + ) + test( + ('foo.md', 'foo/bar'), + exp_true='foo/bar, foo/bar/index.md, foo/bar/README.md, foo/bar.md, foo/foo/bar, foo/foo/bar/index.md, foo/foo/bar/README.md, foo/foo/bar.md', + exp_false='foo/bar, foo/bar/index.md, foo/bar/README.md, foo/bar.md', + ) + test( + ('foo.md', 'foo/bar/'), + exp_true='foo/bar, foo/bar/index.md, foo/bar/README.md, foo/bar.md, foo/foo/bar, foo/foo/bar/index.md, foo/foo/bar/README.md, foo/foo/bar.md', + exp_false='foo/bar, foo/bar/index.md, foo/bar/README.md', + ) + + test( + ('foo.md', 'foo.html'), + exp_true='foo.html, foo.md, foo/foo.html, foo/foo.md', + exp_false='foo.html, foo.md', + ) + test( + ('foo.md', '../foo/'), + exp_true='../foo, foo, foo/index.md, foo/README.md, foo.md', + exp_false='../foo', + ) + test( + ('foo.md', 'foo/'), + exp_true='foo, foo/index.md, foo/README.md, foo.md, foo/foo, foo/foo/index.md, foo/foo/README.md, foo/foo.md', + exp_false='foo, foo/index.md, foo/README.md', + ) + test(('foo/index.md', 'index.md'), expected='foo/index.md') + test(('foo/index.md', 'foo/bar.html'), expected='foo/foo/bar.html, foo/foo/bar.md') + test(('foo/index.md', '../foo.html'), expected='foo.html, foo.md') + test(('foo/index.md', '../'), expected='., index.md, README.md') + test( + ('foo/bar.md', 'index.md'), + exp_true='foo/index.md, foo/bar/index.md', + exp_false='foo/index.md', + ) + test( + ('foo/bar.md', 'foo.md'), + exp_true='foo/foo.md, foo/bar/foo.md', + exp_false='foo/foo.md', + ) + test( + ('foo/bar.md', 'bar.md'), + exp_true='foo/bar.md, foo/bar/bar.md', + exp_false='foo/bar.md', + ) + test( + ('foo/bar.md', 'foo/bar.md'), + exp_true='foo/foo/bar.md, foo/bar/foo/bar.md', + exp_false='foo/foo/bar.md', + ) + test( + ('foo/bar.md', 'foo'), + exp_true='foo/foo, foo/foo/index.md, foo/foo/README.md, foo/foo.md, foo/bar/foo, foo/bar/foo/index.md, foo/bar/foo/README.md, foo/bar/foo.md', + exp_false='foo/foo, foo/foo/index.md, foo/foo/README.md, foo/foo.md', + ) + test( + ('foo/bar.md', 'foo/bar'), + exp_true='foo/foo/bar, foo/foo/bar/index.md, foo/foo/bar/README.md, foo/foo/bar.md, foo/bar/foo/bar, foo/bar/foo/bar/index.md, foo/bar/foo/bar/README.md, foo/bar/foo/bar.md', + exp_false='foo/foo/bar, foo/foo/bar/index.md, foo/foo/bar/README.md, foo/foo/bar.md', + ) + test( + ('foo/bar.md', 'foo.html'), + exp_true='foo/foo.html, foo/foo.md, foo/bar/foo.html, foo/bar/foo.md', + exp_false='foo/foo.html, foo/foo.md', + ) + test( + ('foo/bar.md', 'foo/bar.html'), + exp_true='foo/foo/bar.html, foo/foo/bar.md, foo/bar/foo/bar.html, foo/bar/foo/bar.md', + exp_false='foo/foo/bar.html, foo/foo/bar.md', + ) + test( + ('foo/bar.md', '../foo/bar.html'), + exp_true='foo/bar.html, foo/bar.md, foo/foo/bar.html, foo/foo/bar.md', + exp_false='foo/bar.html, foo/bar.md', + ) + test( + ('foo/bar.md', '../foo'), + exp_true='foo, foo/index.md, foo/README.md, foo.md, foo/foo, foo/foo/index.md, foo/foo/README.md, foo/foo.md', + exp_false='foo, foo/index.md, foo/README.md, foo.md', + ) + test( + ('foo/bar.md', '../'), + exp_true='., index.md, README.md, foo, foo/index.md, foo/README.md', + exp_false='., index.md, README.md', + ) + + for src in 'foo/bar.md', 'foo.md', 'foo/index.md': + test((src, '/foo'), expected='foo, foo/index.md, foo/README.md, foo.md') + test((src, '/foo/bar.md'), expected='foo/bar.md') + test((src, '/foo/bar.html'), expected='foo/bar.html, foo/bar.md') + + for dest in '', '.', './': + test(('index.md', dest), expected='., index.md') + test(('foo/bar.md', dest), expected='foo, foo/bar.md') + + test( + ('foo/bar.md', '../test.png'), + exp_true='test.png, test.png.md, foo/test.png, foo/test.png.md', + exp_false='test.png, test.png.md', + ) diff --git a/mkdocs/utils/__init__.py b/mkdocs/utils/__init__.py index 4787d8ca..7866bdd6 100644 --- a/mkdocs/utils/__init__.py +++ b/mkdocs/utils/__init__.py @@ -92,6 +92,16 @@ def get_build_date() -> str: return get_build_datetime().strftime('%Y-%m-%d') +if sys.version_info >= (3, 9): + _removesuffix = str.removesuffix +else: + + def _removesuffix(s: str, suffix: str) -> str: + if suffix and s.endswith(suffix): + return s[: -len(suffix)] + return s + + def reduce_list(data_set: Iterable[T]) -> list[T]: """Reduce duplicate items in a list and preserve order""" return list(dict.fromkeys(data_set))