-
Notifications
You must be signed in to change notification settings - Fork 10
docs: build relative path to preview examples #937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| import re | ||
| import xml.etree.ElementTree as etree | ||
| from abc import abstractmethod | ||
| from pathlib import Path | ||
| from typing import cast | ||
| from urllib.parse import urlencode | ||
| from urllib.parse import urlencode, urlparse | ||
|
|
||
| from markdown import Extension | ||
| from markdown.preprocessors import Preprocessor | ||
|
|
@@ -96,9 +97,18 @@ def convert_tag(self, line) -> str: | |
| pass | ||
|
|
||
| class ElementExamplePreProcessor(ElementHtmlPreProcessor): | ||
| def __init__(self, examples_base, *args, **kwargs): | ||
| def __init__(self, examples_base, page, *args, **kwargs): | ||
| """Initialize.""" | ||
| self.examples_base = examples_base | ||
| self.page = page | ||
| url = urlparse(self.examples_base) | ||
| # If no scheme, we assume we need to build a relative path to the examples | ||
| if url.scheme == '': | ||
| # Number of non-empty segments in /foo/bar/ → 2, used as "../" depth | ||
| depth = len([s for s in self.page.strip('/').split('/') if s]) | ||
| rel_prefix = '../' * depth | ||
| self.examples_base = f'{rel_prefix}{self.examples_base.lstrip("/")}' | ||
|
|
||
| super().__init__('si-docs-component', *args, **kwargs) | ||
|
|
||
| def convert_tag(self, line) -> str: | ||
|
|
@@ -132,13 +142,21 @@ class ElementDocsExtension(Extension): | |
| def __init__(self, *args, **kwargs): | ||
| """Initialize.""" | ||
| self.config = { | ||
| 'examples_base': ['', 'Base URL for the examples.'] | ||
| 'examples_base': ['', 'Base URL for the examples.'], | ||
| 'md_file': ['', 'Helper for accessing the MkDocs config (set via !relative).'] | ||
| } | ||
| super().__init__(*args, **kwargs) | ||
|
|
||
| def extendMarkdown(self, md): | ||
| """Add Tabbed to Markdown instance.""" | ||
| md.preprocessors.register(ElementExamplePreProcessor(self.config.get('examples_base')[0], md), 'element_example', 10) | ||
| # Extract current page path | ||
| md_file_cfg = self.config.get('md_file')[0] | ||
| mkdocs_config = getattr(md_file_cfg, 'config', None) | ||
| current_page = getattr(mkdocs_config, '_current_page', None) | ||
| page = getattr(current_page, 'abs_url', '') or '' | ||
| if hasattr(current_page, 'abs_url'): | ||
| page = current_page.abs_url | ||
| md.preprocessors.register(ElementExamplePreProcessor(self.config.get('examples_base')[0], page, md), 'element_example', 10) | ||
|
Comment on lines
+152
to
+159
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix potential Two issues in the page extraction logic:
Apply this diff to fix both issues: # Extract current page path
-md_file_cfg = self.config.get('md_file')[0]
+md_file_list = self.config.get('md_file', [''])
+md_file_cfg = md_file_list[0] if md_file_list else ''
mkdocs_config = getattr(md_file_cfg, 'config', None)
current_page = getattr(mkdocs_config, '_current_page', None)
page = getattr(current_page, 'abs_url', '') or ''
-if hasattr(current_page, 'abs_url'):
- page = current_page.abs_url
md.preprocessors.register(ElementExamplePreProcessor(self.config.get('examples_base')[0], page, md), 'element_example', 10)🤖 Prompt for AI Agents |
||
| md.treeprocessors.register(ElementTabTreeProcessor(md), 'element_tabs', 10) | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: siemens/element
Length of output: 4506
Based on my verification, I can now provide the rewritten review comment. The original concern is valid and confirmed. The code comments at lines 130-131 explicitly mention the case where examples_base could be
../../../demo/index.htmlon a server, but the code doesn't normalize this input before prepending additional../segments based on page depth.Relative path double-prepending is possible with the current logic.
The code at lines 104–110 will prepend additional
../segments to anyexamples_basewithout a URL scheme. However, ifexamples_baseis already a relative path (e.g.,../../../demo/index.html, as mentioned in the comments at lines 130–131), this results in double-prepending. For example, a page at/docs/reference/foo/(depth 3) would transform../../../demo/index.htmlinto../../../../../../../../../demo/index.html, potentially overshooting the target.To fix this, either:
examples_baseby removing leading../before applying the page-relative prefix, orexamples_basemust be an absolute path (e.g.,/demo/index.html) or a bare relative path without../(e.g.,demo/index.html), or🤖 Prompt for AI Agents