Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

docs: build relative path to preview examples #937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
196d6fc
docs: build relative path to preview examples
spliffone Oct 31, 2025
cae9512
chore: squash
spliffone Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions mkdocs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ markdown_extensions:
- md_in_html
- element-docs:
examples_base: !ENV [ EXAMPLES_BASE, 'http://localhost:4200' ]
md_file: !relative
- toc:
permalink: true

Expand Down
26 changes: 22 additions & 4 deletions projects/element-docs-builder/md_extension_element_docs/extension.py
View file
Open in desktop
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
Expand Down Expand Up @@ -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("/")}'
Comment on lines +104 to +110
Copy link

@coderabbitai coderabbitai bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for examples_base configuration in YAML files and check for relative paths

# Find all YAML files and search for examples_base
rg -n "examples_base" --type yaml -A 2 -B 2

# Also search in any Python config files
rg -n "examples_base" --type py -A 2 -B 2 | head -50

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.html on 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 any examples_base without a URL scheme. However, if examples_base is 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.html into ../../../../../../../../../demo/index.html, potentially overshooting the target.

To fix this, either:

  • Normalize examples_base by removing leading ../ before applying the page-relative prefix, or
  • Document that examples_base must be an absolute path (e.g., /demo/index.html) or a bare relative path without ../ (e.g., demo/index.html), or
  • Use a proper path normalization library to resolve the final path correctly.
🤖 Prompt for AI Agents 
In projects/element-docs-builder/md_extension_element_docs/extension.py around
lines 104-110, the code prepends a page-relative "../" prefix to examples_base
without handling cases where examples_base already starts with "../", causing
double-prepending; fix it by normalizing examples_base before prefixing—either
strip any leading "../" segments from self.examples_base (or convert leading
"./" to none) then prepend rel_prefix, or resolve the final path using a POSIX
path normalization/join (e.g., posixpath.join + posixpath.normpath) so multiple
"../" segments collapse correctly and the resulting relative path is normalized.


super().__init__('si-docs-component', *args, **kwargs)

def convert_tag(self, line) -> str:
Expand Down Expand Up @@ -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
Copy link

@coderabbitai coderabbitai bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix potential IndexError and remove redundant logic.

Two issues in the page extraction logic:

  1. Line 153 could raise IndexError if md_file is configured as an empty list. While unlikely in normal usage, defensive programming would check the list length first.

  2. Lines 156-158 contain redundant logic. Line 156 already extracts abs_url with a fallback to empty string using getattr. Lines 157-158 then repeat the same extraction with hasattr, overwriting the result. The second check is unnecessary.

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 
In projects/element-docs-builder/md_extension_element_docs/extension.py around
lines 152 to 159, the code currently indexes self.config.get('md_file')[0]
without checking the list length and redundantly re-reads abs_url; change it to
first safely retrieve the md_file list, verify it contains at least one element
and bail or use a safe default if empty, then use mkdocs_config =
getattr(md_file_cfg, 'config', None) and current_page = getattr(mkdocs_config,
'_current_page', None) and set page = getattr(current_page, 'abs_url', '')
(remove the subsequent hasattr block), and finally register the preprocessor
using the safe page variable; ensure you do not assume md_file is non-empty to
avoid IndexError.

md.treeprocessors.register(ElementTabTreeProcessor(md), 'element_tabs', 10)


Expand Down
Loading