os.system -> subprocess

[achille-fouilleul]

Quoting the python library documentation:

The subprocess module provides more powerful facilities for spawning
new processes and retrieving their results; using that module is
preferable to using this function.

[chopan050]

Since this function no longer returns a str with the TeX command, but instead returns a list[str] containing the command arguments, could we change this function's name to something like prepare_tex_compilation_args or prepare_tex_compilation_command_args?

[achille-fouilleul]

In the synopsys of subprocess.run, the name of the first and only positional argument, args, is a bit misleading because its first item is the command name. I think it follows the C language tradition where the first item of the argument vector (argv) of exec and main(int argc, char *argv[]) is the command name too.
If I saw a function named prepare_tex_compilation_command_args I would assume it returns the arguments only, not the command. I suggest we simply name this function make_tex_compilation_command.

[chopan050]

make_tex_compilation_command sounds fine to me 👍

[chopan050]

Suggested change

	"""Prepares the tex compilation command with all necessary cli flags
	"""Prepares a list of arguments for the TeX compilation command, with all necessary
	CLI flags.

[chopan050]

I can't select the line above this one (:class:`str`), but could you please change that one to :class:`list`[:class:`str`]?

Suggested change

	Compilation command according to given parameters
	A list of arguments for the TeX compilation command, according to the given parameters.

[achille-fouilleul]

Updating the type annotation, thanks. But I think the description is accurate. What is returned is really the full command, just in list form.

[achille-fouilleul]

There must be an error in this part:

:class:`list`[:class:`str`]

The html output does not look correct in Firefox, something like list`[:class:`str]

[achille-fouilleul]

The correct syntax seems to be:

:class:`list[str]`

[chopan050]

Right, I forgot that there was an issue with :class:`list`[:class:`str`].

The way Sphinx and Autodoc work for us is weird. When it autoinfers the return type from the typings, it doesn't seem to enclose the names with backticks, so if it autoinferred it would simply render list[str]. We do have a lot of explicit return types in docstrings enclosed in backticks, and they're rendered as inline code. That's an inconsistency which we should address at some point.

I would say, for the purpose of this PR, simply using list[str] would be fine.

[chopan050]

Thanks for your PR! I left some very minor changes regarding tex_compilation_command and its docstring.

[achille-fouilleul]

To avoid confusion I renamed variable args to command.

[chopan050]

LGTM!