Skip to content

docs: Remove leading "sphinx-build" from buildArguments setting#1111

Merged
alcarney merged 1 commit into
swyddfa:developfrom
renato-pipkin:develop
May 10, 2026
Merged

docs: Remove leading "sphinx-build" from buildArguments setting#1111
alcarney merged 1 commit into
swyddfa:developfrom
renato-pipkin:develop

Conversation

@renato-pipkin
Copy link
Copy Markdown
Contributor

cd0d7c9 introduced the setting buildArguments(part of #1067). Thus,

buildCommand = ["sphinx-build", "-M", "html", "docs", "${defaultBuildDir}", "--nitpicky", "--verbose"]

became

buildArguments = ["sphinx-build", "-M", "html", "docs", "${defaultBuildDir}", "--nitpicky", "--verbose"]

Note however, that the values of this setting are not arguments but a command.
To me the following would be more correct:

buildArguments = ["-M", "html", "docs", "${defaultBuildDir}", "--nitpicky", "--verbose"]

Note the missing program sphinx-build in the first position, i.e. all the values are arguments.

Luckily, Esbonio already treats it that way:

@classmethod
def fromcli(cls, args: list[str]):
"""Return the ``SphinxConfig`` instance that's equivalent to the given arguments.
Parameters
----------
args
The cli arguments you would normally pass to ``sphinx-build``
Returns
-------
Optional[SphinxConfig]
``None`` if the arguments could not be parsed, otherwise the set configuration
options derived from the sphinx build command.
"""
if args[0] == "sphinx-build":
args = args[1:]

(at least I believe that is the location where the option is used and a potential leading sphinx-build is stripped)

The test suite also seems to cover this use case:

@pytest.mark.parametrize(
"value, expected",
[
({}, SphinxConfig()),
(
{"pythonCommand": ["/bin/python"]},
SphinxConfig(python_command=SubProcess(["/bin/python"])),
),
(
{"pythonCommand": {"command": ["/bin/python"]}},
SphinxConfig(python_command=SubProcess(["/bin/python"])),
),
(
{"buildCommand": ["-M", "html", "src", "dest"]},
SphinxConfig(build_command=["-M", "html", "src", "dest"]),
),
(
{"buildArguments": ["-M", "html", "src", "dest"]},
SphinxConfig(build_command=["-M", "html", "src", "dest"]),
),
],
)
def test_structure_config(value: dict[str, Any], expected: SphinxConfig):

and

https://github.com/swyddfa/esbonio/blob/7d14993a594204371c0c66f90f44ab17f1639c04/lib/esbonio/tests/sphinx-agent/test_sa_unit.py

This pull request, updates the documentation, to reflect that buildArguments are only arguments and buildCommand is the full command (not changed in this PR).


Sidenote:
I noticed there are these two folders:

  • lib/esbonio/tests/server/feature
  • lib/esbonio/tests/server/features

should these be the same?

As the name suggest buildArguments are arguments to the sphinx-build
command.  Strip a leading "sphinx-build" to reflect this.

Background: buildArguments was previously named buildCommand but
was renamed in commit cd0d7c9 to
reflect its usage as arguments to sphinx-build.  buildCommand was
kept as an alias.  Currently, esbonio supports Esbonio currently
supports buildArguments with and without leading "sphinx-build".
Note that internally the arguments are not passed to the (external)
command "sphinx-build" but to the Sphinx API.
@alcarney
Copy link
Copy Markdown
Member

Thank you!

I agree, now that we have buildArguments it's clearer to drop the sphinx-build part.


Sidenote:
I noticed there are these two folders ... should these be the same?

No they should be separate... though it's probably another poor naming choice on my part! 😅

The esbonio server is composed of multiple LanguageFeatures and there is a thin framework that sits on top of pygls to support aggregating response data from multiple features. The (admittedly few!) unit tests for that framework are in tests/server/feature

Unit tests for specific feature implementations, say converting rst directives to completion items live in tests/server/features/

@alcarney alcarney merged commit cb159dc into swyddfa:develop May 10, 2026
4 checks passed
@renato-pipkin
Copy link
Copy Markdown
Contributor Author

Sidenote:
I noticed there are these two folders ... should these be the same?

No they should be separate [...]

Ok, thanks for explaining and merging!

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.

2 participants