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

Links to images are absolute -> does not work in docker #32

Closed
Lazzaretti opened this issue Mar 27, 2023 · 12 comments · Fixed by #46
Closed

Links to images are absolute -> does not work in docker #32

Lazzaretti opened this issue Mar 27, 2023 · 12 comments · Fixed by #46

Comments

@Lazzaretti
Copy link

I use the DownloadImages feature to serve static content. I'm using mkdocs inside a docker container and therefor run mkdocs serve -a 0.0.0.0:8000 .
Unfortunately, the links to the generated diagrams point to http://0.0.0.0:8000/images/kroki_generated/...

Is it possible to use relative paths for the images?

I'm using mkdocs-kroki-plugin==0.6.1

@llibeohs
Copy link

you need to set DownloadDir , Its default value is images/kroki_generated.

@llibeohs
Copy link

plugin.py need some change

self._site_url = config.get("site_url", "/") # find this line 
# I added the following 2 lines of code
if self._site_url is None:
            self._site_url = "/"

@Lazzaretti
Copy link
Author

you need to set DownloadDir , Its default value is images/kroki_generated.

I set it to DownloadDir: kroki but it does not help. Then it will just point to http://0.0.0.0:8000/kroki/...

Lazzaretti pushed a commit to Lazzaretti/mkdocs-kroki-plugin that referenced this issue Mar 28, 2023
@Lazzaretti
Copy link
Author

plugin.py need some change

self._site_url = config.get("site_url", "/") # find this line 
# I added the following 2 lines of code
if self._site_url is None:
            self._site_url = "/"

Unfortunately, this does not work, as the _site_url is set because of the parameter -a 0.0.0.0:8000

@llibeohs
Copy link

your pull request will fix this issue. 😄

@ITopiaJesseGoerzen
Copy link
Contributor

Relative URLs would be preferable.

@ITopiaJesseGoerzen
Copy link
Contributor

  • Eg.
    • The root of the site is hosted on www.example.com/en/
    • The .md / .html file is at www.example.com/en/sub/folder/page.html
    • Kroki generates the actual .svg at www.example.com/en/assets/images/kroki/example123.svg
    • Kroki sets the url of the generated image to www.example.com/assets/images/kroki/example123.svg (which is wrong)
    • The ideal url to the image (from the given page) would be ../../assets/images/kroki/example123.svg

The same issue happens if you try to open the site as a file ( a significant issue if you're trying to automatically generate pdfs from the pages, like me )

@llibeohs
Copy link

plugin.py I made some temporary changes to the code , it may solve your issue @ITopiaJesseGoerzen .

DownloadImages: True
DownloadDir: images
.
├── docs
│   ├── index.md
│   ├── linux
│   │   └── bash
│   │       └── sed.md 
└── site                   # mkdocs build output dir
    ├── 404.html
    ├── about
    │   └── index.html
    ├── index.html
    ├── linux
    │   └── bash
    │       └── sed
    │           ├── images    # <------ will copy to this path
    │           │   └── sed-68ddc10decadb0659e2f4298b6e1dac7.svg
    │           └── index.html
    def _download_dir(self):
        return Path(self._tmp_dir.name) 
   def _save_kroki_image_and_get_url(self, file_name, image_data, files, page):            # <-------
        downloadPath = self._download_dir() / page.url / Path(self.config["DownloadDir"])   # <-------
        filepath = downloadPath / file_name                 # <-------
        os.makedirs(downloadPath ,exist_ok = True)  # <-------
        with open(filepath, 'wb') as file:
            file.write(image_data)
        get_url = relpath(filepath, self._tmp_dir.name)

        mkdocs_file = File(get_url, self._tmp_dir.name, self._output_dir, False)
        files.append(mkdocs_file)

        return Path(self.config["DownloadDir"]) / file_name   # <------- use real relative path
  def _replace_kroki_block(self, match_obj, files, page):
      ....
      if image_data:
                file_name = self._kroki_filename(kroki_data, kroki_type, page)
                get_url = self._save_kroki_image_and_get_url(file_name, image_data, files, page)  # <-------

@b-bittner
Copy link
Contributor

First of all, thanks ALL for the great collaboration!

I would prefer a solution that always works (without a configuration option like in #33 ).
But I'm not quite sure if the idea from @llibeohs works in all scenarios.

Feedback from others are welcome:
What do you think, would this solve the problem and also meet all requirements?

@jortkoopmans
Copy link

Thank you @oniboni : I have tested the module on your PR branch (PR #46) and this works perfectly to resolve this issue.
Now the images are stored in the same directory as the markdown files for each (sub)section and are correctly referenced in the html.
Note I have used the following settings:

site_url: ""
  # [..]
plugins:
  # [..]
  - kroki:
      HttpMethod: GET
      DownloadImages: True

However, one thing I wasn't sure about, you mention in your PR that you deprecate the DownloadDir option, but when I have this option it crashed with an error for me:

ERROR - Config value 'plugins': Plugin 'kroki' option 'DownloadDir': The configuration option 'DownloadDir' was removed from MkDocs.

If my test is correct this would mean a breaking change instead of deprecating.

@oniboni
Copy link
Collaborator

oniboni commented Nov 10, 2023

@jortkoopmans The DownloadDir option is obsolete with the new approach. Therefore this is indeed a breaking change in terms of the plugin configuration. The option is mapped via Deprecated. Since it does not have a move_to target (because it is obsolete) an error gets raised, because the config has to be fixed.

Any suggestions?

@jortkoopmans
Copy link

The ideal status of the config option would be: abandoned but not removed (yet) for backward compatibility. But this status currently does not exist it seems.

It seems there is no way to truly remove the config option without raising the validation error. This line I think
So I think your solution is preferred, as long as we clearly communicate the config change is breaking.

Alternatively, not removing the option and just deprecating with a message could be acceptable. However it could lead to confusion as the option is not used anymore (and therefore does not fit deprecated status).

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