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.

Sign up for GitHub

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

Jump to bottom

Run sphinx-autobuild via tox -e docs-live[-src] #2400

Merged
merged 2 commits into from Feb 18, 2024
Merged

Run sphinx-autobuild via tox -e docs-live[-src] #2400

merged 2 commits into from Feb 18, 2024

Conversation

rmartin16
Copy link
Member

Changes

  • Adds support to run sphinx-autobuild from tox
  • Running tox -e docs-live will incrementally build any detected changes within the docs directory
  • Running tox -e docs-live-src will include rebuilding for changes to docstrings in the source that are compiled in to the documentation....but this requires a full rebuild of the docs for any changes since Sphinx doesn't have a mapping of source files to docs files.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@rmartin16
Copy link
Member Author

rmartin16 commented Feb 16, 2024 •
edited

Do the docs use docstrings from elsewhere than core/src/toga/?

@HalfWhitt
Copy link
Contributor

Oh hell yes. One further idea / request: it looks like there's also an --open-browser flag, so you don't even have to manually copy the local address to your browser. Any objection to including that too?

@HalfWhitt
Copy link
Contributor

In my usage so far, this seems confusingly inconsistent. It always registers a file save in the console, but sometimes the changes are updated in the browser and sometimes they aren't. I haven't yet been able to determine a rhyme or reason to it.

@freakboy3742
Copy link
Member

freakboy3742 commented Feb 16, 2024 •
edited

Do the docs use docstrings from elsewhere than core/src/toga/?

No - the public docs should only be dependent on the toga-core package.

Oh hell yes. One further idea / request: it looks like there's also an --open-browser flag, so you don't even have to manually copy the local address to your browser. Any objection to including that too?

That seems like reasonable thing to turn on. Alternatively (or as well), you could configure the tox target so that it allows passthrough of command line arguments (see how the tox -e py target is configured) so that tox -e docs-live -- --open-browser would be legal. I'll leave it up to you @rmartin16 to make a call on which one feels better as a user.

@rmartin16
Copy link
Member Author

After experimenting with --open-browser, I'm definitely hesitant to invoke it by default. Initially, just adding --open-browser in tox caused it to open the docs in Lynx...which is a fun way to browse web pages (although stdin was getting mangled somehow so it didn't even work). I eventually intuited that the webbrowser module must not think I have X running....so, I passed through the DISPLAY environment variable and got a error saying Firefox basically wasn't cooperating and the page cannot be opened. On Windows, the experience went better and Edge opened.

So, I'm inclined to leave the eccentricities of webbrowser out while allowing anyone to use it via positional arguments at the command line. Furthermore, the hyperlink shown after the docs are built should be control-clickable in any competent terminal.

I also added --port 0 to avoid port conflicts since 8000 is the default and a popular one at that.

@rmartin16
Copy link
Member Author

Scratch the random port; it's too annoying if you restart autobuild and expect the previous browser tab you were using to work again.

As for the inconsistencies you noted, @HalfWhitt, I've been seeing some bizarre things as well. Lacking intimate knowledge of how Sphinx works....my assumption is the reuse of the _build/html directory is leading to a strange mix of new and outdated pages somehow.

@rmartin16 rmartin16 force-pushed the tox-live-docs branch 3 times, most recently from b3be34a to da3e0d8 Compare February 16, 2024 19:20
rmartin16
rmartin16 commented Feb 16, 2024
View reviewed changes
tox.ini Outdated
Comment on lines 39 to 43
[docs]
build_dir = _build
# -W: make warnings into errors
# --keep-going: continue on errors
# -j: run with multiple processes
sphinx_args = -W --keep-going -j auto
# -v: verbose logging
# -E: force rebuild of environment
# -T: print traceback on error
# -a: read/parse all files
# -d: use tox's temp dir for caching
sphinx_args_extra = {[docs]sphinx_args} -v -E -T -a -d {envtmpdir}/doctrees
# replace when Sphinx 7.3 is released:
# -T -> --show-traceback
# -W -> --fail-on-warning
# -b -> --builder
# -v -> --verbose
# -a -> --write-all
# -E -> --fresh-env
sphinx_args = -T -W --keep-going --jobs auto
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to re-work all of this from my original implementation.

Using -E, -a, and -d {envtmpdir}/doctrees all the time makes all of this really slow since Sphinx basically needs to start from scratch every time. In your local environment, though, using all the caching really speeds things up. Use tox -e docs -- -aE if you want to start from scratch locally; tox -e docs-all will continue always re-reading everything, though.

Using -v in a terminal creates a worse experience since it needlessly lists all the files being read instead of using the screen redrawing to list the files being processed.

@HalfWhitt
Copy link
Contributor

HalfWhitt commented Feb 17, 2024 •
edited

I wonder if the inconsistent loading has anything to do with the fact that tox doesn't seem to be installing core as editable; that's probably why deleting the relevant environment folder in .tox always updates docstrings, since the packages are all reinstalled from their current state.

EDIT: Eh, never mind. Changing that doesn't seem to make it any more reliable.

@rmartin16
Copy link
Member Author

I wonder if the inconsistent loading has anything to do with the fact that tox doesn't seem to be installing core as editable; that's probably why deleting the relevant environment folder in .tox always updates docstrings, since the packages are all reinstalled from their current state.

Actually, that's a good point; I think all my testing of seeing edits in the "live view" was while running the command without tox so I could get the right command line.

That's why the usual recommendation for updating API docstrings in your local build was to run tox -re docs-all...which is the equivalent of deleting the .tox directory but a little cleaner.

At any rate, I'll have it install an editable version....that should be fine since its essentially what's happening when I run sphinx directly in the command line.

@rmartin16
Copy link
Member Author

EDIT: Eh, never mind. Changing that doesn't seem to make it any more reliable.

As for other inconsistencies, some examples may help since using a different build directory seemed to make everything a lot better for me.

@rmartin16
Copy link
Member Author

I also realized the docs-live-src tox env needed the -E/--fresh-env flag so it didn't re-use the cached docstrings.

@rmartin16
Copy link
Member Author

FYI; beware of using {/} as a trailing path separator; it doesn't work that well on Windows right now.

@rmartin16 rmartin16 mentioned this pull request Feb 17, 2024
Merged
4 tasks
freakboy3742
freakboy3742 approved these changes Feb 18, 2024
View reviewed changes
Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huge +1 on this. I've added a reference to the contribution guide so that everyone knows this exists.

@freakboy3742 freakboy3742 merged commit 83ef374 into beeware:main Feb 18, 2024
35 checks passed
@rmartin16 rmartin16 deleted the tox-live-docs branch February 18, 2024 08:06
@HalfWhitt
Copy link
Contributor

Thank you so much for implementing this rmartin16, you rock! The merged version is working beautifully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@freakboy3742 freakboy3742 freakboy3742 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rmartin16 @HalfWhitt @freakboy3742