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

LaTeX refactoring, use \RequirePackage always, not \input #12703

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

jfbu
Copy link
Contributor

@jfbu jfbu commented Jul 28, 2024

  • Refactoring

At Sphinx 4.0.0 the file sphinx.sty was split into multiple smaller files.

Some of them structured as packages others as files using \ProvidesFile.

I am currently working on some LaTeX PR, and this situation is a slight annoyance. Files with using \ProvidesFile can only be \input and then only once (due to \newcommand which can not be executed twice). This is sometimes a problem when for some reason we want to indicate in one file that its macros require some defined in some other file. In TeX, macros do not have to be defined beforehand to be used in other macro definitions, but abusing this is no good for future maintenance.

Using \RequirePackage is a good way, because we can do it multiple times and it will not complain and will actually load the required file only once. And A can require B while B requires A. No problem about that in LaTeX. Here Package A requires B which is itself a package requiring A. Nothing bad happens:

 (./A.sty
Package: A 2024/07/28 Package A

(./B.sty
Package: B 2024/07/28 Package B
))

Using \ProvidesFile and not \ProvidesPackage was a mistake.

This fixes it.

Only people (do they exist at all?) having really messed around with replacing some of those files with custom ones will have to adjust and also use \ProvidesPackage in their own (surely less good) variants of Sphinx own perfect files.

I think a simple addition to CHANGES will be enough to signal some internal change.

@jfbu jfbu added this to the 8.x milestone Jul 28, 2024
@jfbu
Copy link
Contributor Author

jfbu commented Jul 31, 2024

@AA-Turner Thanks for review. This is basically internal refactoring, do you think it deserves some CHANGES entry, and if yes where?

@AA-Turner
Copy link
Member

For refactoring I tend not to do a CHANGES entry, as it is purely internal.

A

@jfbu
Copy link
Contributor Author

jfbu commented Jul 31, 2024

Ok, thanks for advice. I trust nobody has really completely overwritten any of the .sty file with one's own, if yes, they need to replace also \ProvidesFile by \ProvidesPackage line. Hopefully people will find this comment in case of need. I will merge now.

@jfbu jfbu merged commit 97391b4 into sphinx-doc:master Jul 31, 2024
19 checks passed
@jfbu jfbu deleted the latex_refactor_assets branch August 7, 2024 20:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2024
@AA-Turner AA-Turner modified the milestones: 8.x, 8.1.0 Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants