Skip to content
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

Including markdown is buggy and local images do not work #691

Closed
meghprkh opened this issue May 15, 2024 · 4 comments · Fixed by #692
Closed

Including markdown is buggy and local images do not work #691

meghprkh opened this issue May 15, 2024 · 4 comments · Fixed by #692
Labels

Comments

@meghprkh
Copy link
Contributor

meghprkh commented May 15, 2024

Problem Description

If you include a markdown and try to link a local image it does not work

A clear and concise description of what the bug is.

Steps to reproduce the behavior:

  1. Create doc/img.png
  2. Create a doc/include.md with an image link ![img](./img.png)
  3. .. include ../doc/include.md
  4. The image is a broken link
    1. It works if the link was ./doc/img.png but that is incorrect markdown

System Information

Paste the output of "pdoc --version" here.
14.4.0

@meghprkh meghprkh added the bug label May 15, 2024
@meghprkh meghprkh changed the title Including markdown is buggy Including markdown is buggy and local images do not work May 15, 2024
@mhils
Copy link
Member

mhils commented May 15, 2024

The logic for determining relative imports sits here:

pdoc/pdoc/docstrings.py

Lines 376 to 383 in 1b36ea2

loc = source_file or Path(".")
try:
included = (loc.parent / val).read_text("utf8", "replace")
except OSError as e:
warnings.warn(f"Cannot include {val!r}: {e}")
included = "\n"
included = _rst_admonitions(included, loc.parent / val)
return indent(included, ind)

The source_file argument should be the path of the current Python file where the docstring is in. We may need an additional call to embed_images here, see further up in the file. I don't have the capacity to dive deeper, but PRs are welcome! 😃

@meghprkh
Copy link
Contributor Author

meghprkh commented May 15, 2024

IMO regex based workarounds are a very bad idea. I might create a patch to address the issue at hand.

While switching to an extensible commonmark compliant markdown parser is a big change, it will remove need of such hacks. This link lists some of them. Markdown-it is fairly extensible.

@mhils
Copy link
Member

mhils commented May 16, 2024

This is a matter of setting the relative path correctly, not a matter of regexes or the markdown engine we are using. :)

@meghprkh
Copy link
Contributor Author

I have created #692. Its a minimal fix but should work for the meanwhile :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants