Conversation
| ) | ||
|
|
||
| return anchor_mapping | ||
| return anchor_mapping, src_py_path_mapping |
There was a problem hiding this comment.
src_py_path_mapping is used to know wich mdx file to update/build when a docstring is updated inside python file
There was a problem hiding this comment.
Just want a different name for it, but completely okay with this addition :-)
sgugger
left a comment
There was a problem hiding this comment.
Looks like a great addition, thanks for working on this! I have a few comments on the variable names and we need to handle watchdog as a soft dependency (that the user should install only if they want to use doc-builder dev).
The name dev is not super intuitive to me, is it something that comes from node? I'd prefer something like doc-builder preview (or if you have another idea I'm all ears!).
| ) | ||
|
|
||
| return anchor_mapping | ||
| return anchor_mapping, src_py_path_mapping |
There was a problem hiding this comment.
Just want a different name for it, but completely okay with this addition :-)
| env["DOCS_LIBRARY"] = args.library_name | ||
| env["DOCS_VERSION"] = args.version | ||
| env["DOCS_LANGUAGE"] = args.language | ||
| Thread(target=start_sveltekit_dev, args=(tmp_dir, env, args)).start() |
There was a problem hiding this comment.
This will never be closed?
There was a problem hiding this comment.
this thread should be running as long as the program is running
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
|
@sgugger thanks a lot for the suggestions!
|
sgugger
left a comment
There was a problem hiding this comment.
Thanks for adding all of that! I just left one last styling suggestion.
For the Thread, are we sure that exiting will close it (it's not the case with Process if I remember correctly, so just want to be 100% sure)
|
|
||
|
|
||
| def preview_command(args): | ||
| if is_watchdog_available(): |
There was a problem hiding this comment.
To avoid everything being indented here, I'd test the negation and raise the ImportError, then just unindent the whole function.
| # files/folders cannot have a name that starts with `__` since it is a reserved Sveltekit keyword | ||
| for dir in output_path.glob("**/__*/*"): | ||
| dir.rmdir() | ||
| subprocess.run(["rm", "-rf", str(dir)]) |
There was a problem hiding this comment.
| subprocess.run(["rm", "-rf", str(dir)]) | |
| shutil.rmtree(dir) |
This is cleaner.
There was a problem hiding this comment.
this glob can catch a file "**/__*/*"
in that case, would shutil.rmtree(dir) work?
There was a problem hiding this comment.
It only works for dir, so you should make a test. It's still better than using subprocess for this.
|
@LysandreJik please feel free to retry the preview command. Fixed the relative path issue (should be working whether you provide absolute or relative path to docs folder) |
|
Thanks for the ping, will try it again. |
Dev command with hot reloading
doc-builder preview datasets ~/Desktop/datasets/docs/sourceI think this will make the experience writing docs better 🙂
letsgo_U79b9L1x.mov