mondeja / mkdocs-include-markdown-plugin

Mkdocs Markdown includer plugin
Apache License 2.0
110 stars 20 forks source link

Exclude mkdocs-gen-files from file watching when including them #231

Closed Tremeschin closed 1 month ago

Tremeschin commented 1 month ago

Hi, thanks for the plugin!

I was going to create an issue but found a quick fix for an issue I'm having. It's just two lines of code, let me explain it:

This small change fixes an interaction between include-markdown and files generated from mkdocs-gen-files. As the former adds blindly all included files to the FileWatcher (perfectly valid approach), this fails with dynamic files from mkdocs-gen-files, as they are written to a temporary directory that gets deleted shortly after.

Example

It goes something like this (main points):

import mkdocs_gen_files

with mkdocs_gen_files.open("project/examples/basic.py", "w") as virtual:
    virtual.write(Path("/path/to/true/basic.py").read_text()

And then on an examples documentation markdown:

{% include-markdown "project/examples/basic.py" %}

When running mkdocs serve, it'll fail with the error below:

No such file or directory: /tmp/mkdocs_gen_files_0xbb02fp/project/examples/basic.py

The fix

The traceback is quite long, but ultimately points to a watchdog.observers failing to find this file; bypassing IncludeMarkdownPlugin._update_watched_files fixes it, and as mkdocs-gen-files will always prefix to a directory starting with "mkdocs_genfiles", simply filtering out those on your function does the trick and builds properly the website!


I'll monkey patch to bypass this function in my code for now until a new release 🙂

mondeja commented 1 month ago

Ahm, it seems that there is:

https://github.com/oprypin/mkdocs-gen-files/blob/7baa03225e6c34cc85d17f79c47e42eb2c2e359e/mkdocs_gen_files/editor.py#L51

Would you mind to change the logic of the files watcher to set this information along with the file name?

Tremeschin commented 1 month ago

seems like a hack

Yup, that is possible and I agree my solution isn't the best, just unlikely to happen 😅. I had just looked on type hints and saw it was list[str] and lost hope trying something better haha

change the logic of the files watcher

Ah, you'll definitely have more expertise than me to quickly implement it. I could try just no guarantees I can do it fast or on a way you'd expect, but I'm taking a look right now, you could edit or just close this PR if you do it!

mondeja commented 1 month ago

Ah, you'll definitely have more expertise than me to quickly implement it. I could try just no guarantees I can do it fast or on a way you'd expect, but I'm taking a look right now, you could edit or just close this PR if you do it!

Thanks. If you provide a repository with a minimal example of your problem that I can clone, I'm sure I can implement it quickly.

Tremeschin commented 1 month ago

Sounds better! let me do it rq~

Tremeschin commented 1 month ago

Here it is: https://github.com/TremeschinArchive/pr231

uhm, half sorry I couldn't reproduce my main repo's case which is the file that's including the gen-files example script with your plugin is also a gen-files markdown 😓..

..so this fails on an early point and bypassing does nothing*

Nevertheless I would kinda expect this to work also? the code seems to reach the point where it does copies the basic.py script, by removing the include-markdown on the index.md it's available under localhost:8000/examples/basic.py

*1 I'm trying to reproduce it but you could take a look at the current code, which is a general case? probably a fix will be in the order stuff happens or "lazily" loading / integrating with gen-files

Tremeschin commented 1 month ago

A, found something interesting and reproduced the original bug I tried to fix

Relative paths within include-markdown plays nicely with mkdocs-gen-files (relative branch), and the original issue is reproduced if the markdown file that's including a mkdocs-gen is also a gen-files (original-issue branch)

I have this weird scenario as I work in a monorepo and copy individual projects docs to a single main website

uhm, I could go with relative paths on my main repo for those np, but having absolute paths on the include would make eventual path changes much easier, so the original issue on the PR is a much broader interaction

mondeja commented 1 month ago

absolute paths on the include would make eventual path changes much easier, so the original issue on the PR is a much broader interaction

I'm pretty sure that you're using here the term "absolute" as relative to the root of your project. If I use a truly absolute path like the next works:

{% include-markdown "/home/path/to/your/pr231/examples/basic.py" %}

The exception in the original issue I suspect that is triggerred by mkdocs-gen-files, not from mkdocs-include-markdown. Note that mkdocs-gen-files uses a custom temporal directory, in the case of the original reproduction is /tmp/mkdocs_gen_files_0xbb02fp/, and you're joining the part project/examples/basic.py using mkdocs_gen_files.open("project/examples/basic.py", "w"), so it becomes a proper error as expected because the file doesn't exists:

No such file or directory:  /tmp/mkdocs_gen_files_0xbb02fp/project/examples/basic.py

How could mkdocs-include-markdown-plugin know about what is the root of your project? By design uses files relative to the docs/ directory (formerly, Mkdocs' docs_dir config option) if the path looks like some/thing.ext, relative to the includer file if the path is a proper relative path like ../some/thing.ext and real absolute paths of your filesystem if the path is an absolute path like /some/thing.ext.

So I can't help you here. Another different request would be to create an option in configuration to set the base directory for some/thing.ext includes, but that wouldn't solve your problem because if your docs/ directory location changes you'll need to update it anyway. And I'm at a first glance against it because it would add strange complexity without a clear benefit.

I'm closing, feel free to reopen if you've any questions about.

Tremeschin commented 1 month ago

Thanks for the insight and attention @mondeja !


you're using here term "absolute" as relative to the root of your project

Exactly! I failed to differentiate true relative (./) and 'implicit' (dir/file) relative to docs_dir, my bad 🙂


The exception in the original issue I suspect that is triggerred by mkdocs-gen-files (...) How could (...) know about what is the root of your project (...)

Yea, I guess one thing to try is having an empty docs_dir and copy+work with all files within mkdocs-gen-files itself, maybe also overriding docs_dir to the temp dir. Or always using file-relative paths on the includes which works fine

There's no culprit here but me haha, half sorry for not making an issue first but we can wrap it here for sure!


So I can't help you here (...) add strange complexity without a clear benefit.

No problem! I knew it was an unique interaction and use case to begin with, and the PR was a short lived experiment. I'll go with relative includes + bypass workaround for now, had noticed later it was a broader issue and it got complex.

Thanks for checking the feasibility of a stabler interaction between the two though. I'll toy around with those options I have, but it's not a breaking feature for me, just a nice-to-have

(react this with a thumbs up if if you'd want me to post a last comment if I get it working in the future!)