Skip to content

fix: use file move in stead of read#30

Closed
watermarkhu wants to merge 2 commits intolandmaj:masterfrom
watermarkhu:patch-1
Closed

fix: use file move in stead of read#30
watermarkhu wants to merge 2 commits intolandmaj:masterfrom
watermarkhu:patch-1

Conversation

@watermarkhu
Copy link
Copy Markdown
Contributor

@watermarkhu watermarkhu commented Mar 30, 2026

Allows the plugin to be used with mkdocs-same-dir. Alternative fix proposed in also oprypin/mkdocs-same-dir#13. But this is a good change nonetheless.

@landmaj
Copy link
Copy Markdown
Owner

landmaj commented Mar 30, 2026

Thanks for the change, however I think it should be fixed in mkdocs-same-dir. The code as it is now works and I would rather not change it to something untested if it does not fix or improve anything. And there is nothing to fix. In MkDocs 1.6 (which is a requirement of this plugin) files can be stored in memory, as specified here. If not with this one, than at some point mkdocs-same-dir will break with some other plugin.

Since MkDocs 1.6 a file may alternatively be stored in memory - content_string/content_bytes.

Then src_dir and abs_src_path will remain None. content_bytes/content_string need to be written to, or populated through the content argument in the constructor.

But src_uri is still populated for such files as well! The virtual file pretends as if it originated from that path in the docs directory, and other values are derived.

@watermarkhu
Copy link
Copy Markdown
Contributor Author

watermarkhu commented Apr 5, 2026

Yeah sure I agree with you that nothing is broken here actually. That is why I've also submitted a pull request to mkdocs-same-dir 😊

Ideally that other PR is accepted. But the owner so far has not responded. Would you still consider this refactor then? I could add an explicit test that checks for the existence of the stylesheet in the build dir.

@landmaj
Copy link
Copy Markdown
Owner

landmaj commented Apr 6, 2026

Sure, I don’t want to block your use case due to principles. I just need to test this manually, which I will do after Easter holidays. So tomorrow or the day after tomorrow.

@landmaj landmaj self-assigned this Apr 9, 2026
@landmaj landmaj added the enhancement New feature or request label Apr 9, 2026
@landmaj
Copy link
Copy Markdown
Owner

landmaj commented Apr 9, 2026

I merged your commits locally since there was an issue with package name passed to importlib.resources.path. It would blow up when running with the plugin installed using -e flag.

Version 1.7.0 with your changes is released.

@landmaj landmaj closed this Apr 9, 2026
@landmaj landmaj removed the enhancement New feature or request label Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants