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.

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

Improve php-src docs sphinx build, also on *nix #16743

Open
wants to merge 4 commits into
base: PHP-8.4
Choose a base branch
from

Conversation

hakre
Copy link
Contributor

@hakre hakre commented Nov 9, 2024

Close to a good share of a hand full of improvements of the php-src docs build.

Driven by my initial request in PHP-DEV for wiki editing right after which @cmb69 pointed me to the fact that there is more value in moving things towards php-src.

Therefore I looked into it and found make html not working addressed by adding a Makefile.

Found a benefit in having the requirements in a file instead of scattered, henceforth added a requirements.txt file and bound it in all places I could find. (the build could perhaps benefit from opening to branches other than master for docs/**, only publishing should remain on master branch only.)

Would be delighted to get a review by @iluuu1994 a) for the overall thing and b) there is a deprecation in the theme configuration and the theme is new to me.

In 19d2b84 ("Create book for docs", 2024-01-30) the build of the
php-src documentation has been introduced.

It is based on reStructuredText (rst) [Docutils] for its source files,
this stems from the sphinx-build utility in use to build the static HTML
pages of the php-src documentation.

The maximum line length of these text files has been set to 100
characters.

This formatting constraint is applied with the rstfmt utility [rstfmt]
via its invocation (documented in CI build instructions and README.md:)

    rstfmt -w 100 source

The `-w, --width` option takes a WIDTH argument that is "the
target line length in characters" (cf. `rstfmt --help`.)

There is also an `--ext EXT` argument option, that is "the extension of
files to look at when passed a directory" ("source" is the name of
a directory in the invocation above) and defaults to "rst".

Henceforth, the editor configuration [EditorConfig] can benefit from
documenting this expectation in the repositories .editorconfig file,
which has been introduced already earlier in 5c38fbe ("Added
editorconfig file", 2016-06-26).

[Docutils]: https://docutils.sourceforge.io/index.html "Docutils: Documentation Utilities — Written in Python, for General- and Special-Purpose Use"
[rstfmt]: https://github.com/dzhu/rstfmt "A formatter for reStructuredText"
[EditorConfig]: https://editorconfig.org/ "EditorConfig helps maintain consistent coding styles for multiple developers working on the same project across various editors and IDEs"
In 19d2b84 ("Create book for docs", 2024-01-30) the php-src
documentation (php-src docs) build has been introduced, yet the build
instructions, namely `make html`, did not yield the expected results
within the parenting setup of the php-src project on *nix systems.

The reason is that the `make html` build instruction does not execute
the make.bat file which contains the recipe to build the static HTML
pages. This is because — different to Windows systems — *nix systems
do not have the concept of [COMSPEC] etc. for executables, and
therefore we cannot expect for build scripts an interpreter for the
batch file.

Adding a Makefile suffices to recover the build of php-src ./docs on a
*nix system.

NOTE: The "help" target has been intentionally left out as this Sphinx
builder [1] (with the same name) is not available in the configuration,
nor in use in CI (.github/workflows/docs.yml), and otherwise appeared
undocumented. Consequentially, the makefile goal does not include it,
as well as a target for any other unspecfied builder.

[COMSPEC]: https://en.wikipedia.org/wiki/COMSPEC "COMSPEC or ComSpec is one of the environment variables used in DOS, OS/2 and Windows, which normally points to the command line interpreter, which is by default COMMAND.COM in DOS,[1] Windows 95, 98, and ME or CMD.EXE in OS/2 and Windows NT. — Wikipedia"
[1]: https://www.sphinx-doc.org/en/master/usage/builders/index.html "Built-in Sphinx builders"
Define the required packages to install for the php-src docs build in
the docs/requirements.txt file:

1) Sphinx
2) sphinx-design
3) sphinxawesome-theme
4) rstfmt

This should also later on ease the use of a requirements_frozen.txt
file to pin the build dependencies if needed/wanted.

Additionally, some formatting corrections in README.md (based on the
profile in .editorconfig) as well as adding the recommendation to use
a Python virtual environment. Python3 and Pip were already named, and
with Python3 there is the venv module (Python 3.3; Sep 2012) to manage
these so-called python virtual environments [venv], which are commonly
a preferred way to install dependencies within development projects
and build systems.

[venv]: https://docs.python.org/3/library/venv.html "venv — Creation of virtual environments — Python documentation"
@hakre hakre requested a review from TimWolla as a code owner November 9, 2024 22:46
For the configured Awesome Sphinx Theme [1] highlighting extension, the
sphinx-build currently yields the following diagnostics:

     WARNING: while setting up extension sphinxawesome_theme.highlighting: \
     You no longer have to include the `sphinxawsome_theme.highlighting` \
     extension. This extension will be removed in the next major release.

(via `make html`, the configuration file is `source/conf.py`.)

The diagnostic message was introduced by sphinxawesome-theme 5.2.0,
  released May 31, 2024. [2], [3]

Removing the extension from the list of extensions in the configuration
file levitates.

No changes to requirements.txt, the extension was transitive as bundled
by the Awesome Sphinx Theme [1], and 5.2.0 deprecates it with the new
feature to "Support `pygments_style_dark` option that allows you to set
a different syntax highlighting scheme in light and dark modes." [3]

[1]: https://sphinxawesome.xyz/ "Awesome Sphinx Theme — Create functional and beautiful websites for your documentation with Sphinx."
[2]: https://pypi.org/project/sphinxawesome-theme/5.2.0/#history
[3]: https://github.com/kai687/sphinxawesome-theme/releases/tag/5.2.0
@cmb69
Copy link
Member

cmb69 commented Nov 10, 2024

Thank you! This looks generally good to me (although I have very limited knowledge about Sphinx), but I'm slightly concerned that the build scripts for Windows and POSIX will diverge in the future. Unfortunately, there is not much common ground; maybe we should add some PHP to the mix? Another option would be to drop the Windows build script altogether; users could then use MSYS2 or Cygwin. That might be a hurdle for some, though.

@hakre
Copy link
Contributor Author

hakre commented Nov 11, 2024

I'm slightly concerned that the build scripts for Windows and POSIX will diverge in the future.

Yes, that is something to have under watch, always, but here I see little danger of that, as the build is driven by Python and Docutils (Sphinx) and is basically a simple command. I have spotted no additional magic in the configuration.

maybe we should add some PHP to the mix?

No, not from my perspective. Preferable scripts should be written in Pyhton for a Docutils based build (Spinx).

That might be a hurdle for some, though.

With these requested changes the build instructions should work for users on both *nix and win systems, the overhead for that so far (one small batch and one small makefile) should not really bug us. And if it does I can offer to build back.

Build instructions remain streamlined:

make html

and that's the most important for maintainability from my end.

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

Hi. Thanks for working on improving the docs.

Related: #16676

This can target master, we're not planning to maintain different versions of the document. Differences between versions can be referenced within the document itself.

Another option would be to drop the Windows build script altogether

I agree with this. Note that I did not add this bat file, it ships by default in the sphinx template. If you're developing on Windows, wsl and cygwin seem like good options.

@@ -16,7 +16,6 @@
author = 'The PHP Group'
extensions = [
'sphinx_design',
'sphinxawesome_theme.highlighting',
Copy link
Member

Choose a reason for hiding this comment

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

Why is this dropped?

html : $(SOURCEDIR)/.~
$(SPHINXBUILD) -M $@ $(SOURCEDIR) $(BUILDDIR)
@printf 'Browse the php-src docs \e]8;;%s\e\\%s\e]8;;\e\\ (or \e]8;;%s\e\\%s\e]8;;\e\\)\n' \
"file://$(abspath $(BUILDDIR))/$@/index.$@" "local" "$(GH_PAGES_URL)" "online"
Copy link
Member

Choose a reason for hiding this comment

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

Why do we spell out the online version? That makes it sound like it was deployed.

@@ -19,7 +19,7 @@ jobs:
- name: git checkout
uses: actions/checkout@v4
- name: Install dependencies
run: pip install sphinx-design sphinxawesome-theme rstfmt
run: pip install -r docs/requirements.txt
- name: Check formatting
run: rstfmt --check -w 100 docs/source
Copy link
Member

Choose a reason for hiding this comment

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

This should probably use the make file, to make sure the commands stay consistent.

@cmb69
Copy link
Member

cmb69 commented Nov 14, 2024

Another option would be to drop the Windows build script altogether

I agree with this. Note that I did not add this bat file, it ships by default in the sphinx template. If you're developing on Windows, wsl and cygwin seem like good options.

Yeah, then please drop make.bat.

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.

3 participants