-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add support for autobuilding -M within Sphinx #151
Conversation
When one multiple files within a repository at once (e.g. changing branches for instance), the live view will cause multiple consecutive rebuilds of the documentation set even though it already had built against the new changes. This is especially visible when building the PDF variants of the documentation. This change works well if pyinotify is installed; without pyinotify, this behaves more or less like it did previously. The livereload integration with pyinotify will invoke __call__(self) multiple times in a row with all of the changed files and then the top-level ioloop will invoke our callback once for all of the changed files.
This is useful if one wants to use -M latexpdf for the build environment.
The first patch in this series is probably related to the intent behind the suggestions within #87 although solves it in a different manner that doesn't introduce delays. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given how -M
is implemented I'd prefer to do this avoiding adding -M
to argparse here.
I've just pushed an update which removes LiveReload // Tornado in favour of a simpler WebSocket based implementation, so please remove the tornado imports. They do seem a little out of place, though.
A
self.files = [] | ||
self.ioloop = ioloop.IOLoop.instance() | ||
|
||
def __call__(self): | ||
"""Generate the documentation using ``sphinx``.""" | ||
|
||
if self.watcher.filepath: | ||
show(context=f"Detected change: {self.watcher.filepath}") | ||
self.files.append(self.watcher.filepath) | ||
if len(self.files) == 1: | ||
self.ioloop.add_callback(self.callback) | ||
|
||
def callback(self): | ||
if self.files: | ||
for file in self.files: | ||
if file: | ||
show(context=f"Detected change: {file}") | ||
|
||
self.files = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems unrelated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems unrelated?
It could be split into a separate PR if that'd be helpful. The commit explains it here: 0c2914f
Mainly I've encountered situations where multiple files got touched causing the livereload
layer to call us back multiple times in a row each time causing a build. This is really annoying when building a longer build target, such as when using -M latexpdf
. It's not as annoying with the -b html
target since Sphinx doesn't rebuild the entire doctree for that target (but only the changed files). The -b latex
(and thus also -M latexpdf
) causes the entire doctree to be rebuilt and is terribly slow if that occurs.
When pyinotify
is installed, the callbacks from tornado occur back to back in a row and if we install a callback handler via the tornado ioloop it'll get invoked after all of the callbacks that were generated at the same time. This doesn't work for the file polling method of livereload
(e.g. when pyinotify
is not installed) but it doesn't introduce any adverse behaviors by adding this callback hook at the top level.
I'll look at refactoring this sometime later this week if I get a chance.
I'll take a look at this change. |
sphinx-build
has a hiddenish entry point (it doesn't show up insphinx-build --help
) that is used for some build targets (see https://github.com/sphinx-doc/sphinx/blob/master/sphinx/cmd/build.py#L381-L384). It's useful to be able to autobuild these, like-M latexpdf
, beyond autobuilding the HTML targets. There are PDF viewing programs that will autoreload when the PDF underneath changes, so this can be very useful for previewing the PDF output when one makes changes.Additionally, the second patch allows multiple file changes to be consolidated into one build invocation of
sphinx-build
(at least while running withpyinotify
installed which is automatically used by thelivereload
package if present). This can occur when changing branches, for instance. This is especially visible when doing alatexpdf
build since it takes quite a bit longer than refreshing individual HTML files and it always fully rebuilds the project instead of individual files like ahtml
build does.