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

Move LATEX_PREAMBLE to separate file nbsphinx.sty #708

Merged
merged 3 commits into from
Feb 11, 2023

Conversation

mgeier
Copy link
Member

@mgeier mgeier commented Feb 7, 2023

This moves all the LaTeX code into a separate file.

The LaTeX code is unchanged, except I have removed all the \makeatletter and \makeatother invocations.

I'm not sure if the \RequirePackage{sphinx} has to be there, but I guess it doesn't hurt, right?

@jfbu If you have time, may I ask you to have a look if the nbsphinx.sty file makes sense in your expert opinion?
The main content didn't change, so no need to check this, it's mostly about the \ProvidesPackage and \RequirePackage.

@mgeier
Copy link
Member Author

mgeier commented Feb 7, 2023

Copy link
Contributor

@jfbu jfbu left a comment

Choose a reason for hiding this comment

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

LGTM, indeed makes for simpler structure.

I looked at the generated PDF and apart from some laments about all those straight code blocks corners which could have been so much prettier with a nice curve ;-) and those booktabs tables without colors ;-) I saw nothing peculiar. Not tested on a separate project but only on nbsphinx itself.

@@ -0,0 +1,187 @@
\ProvidesPackage{nbsphinx}[2023/02/06 v0.1 nbsphinx code cells and more]

\RequirePackage{sphinx}
Copy link
Contributor

Choose a reason for hiding this comment

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

The \usepackage{nbsphinx} is positioned after \usepackage{sphinx} in the preamble so this \RequirePackage is not required (sic), but it does not hurt either.

(do not read this) If for some reason in future LaTeX/PDF builds should be attempted only with some minimal version of Sphinx (more recent say that for HTML builds), you have possibility to add some [YYYY/MM/DD] date requirement here. But it causes LaTeX only to raise a warning, not an error anyhow. A masochist user can use make latexpdf LATEXMKOPTS="-Werror" to cause Latexmk to stop as soon as there is a LaTeX warning.

Copy link
Contributor

Choose a reason for hiding this comment

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

tidbit: that LaTeX packages have a "v" before the version number seems to be a community habit. LaTeX itself only assumes that the string within square brackets after the \ProvidesPackage starts with a date (which formerly had to be in YYYY/MM/DD format but now YYYY-MM-DD is supported). Thedoc package used by package authors to manage a .dtx file with both LaTeX macros and user documentation will assume that after the date and a space there is a version number, then a summary of purpose, which it can extract via its \GetFileInfo, but this not part of LaTeX itself, and LaTeX nowhere stores the version numbers of a packages used in a document, it only stores the whole strings which follow the \ProvidesPackage in the various .sty files.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I forgot to say that LaTeX package authors usually end the summary string within square brackets with their initials inside parentheses: \ProvidesPackage{foo}[YYYY/MM/DD version summary (AE)]for Albert Einstein for example. Totally optional.

\endgroup
\renewcommand*\sphinxbreaksbeforeactivelist{\do\<\do\"\do\'}
\renewcommand*\sphinxbreaksafteractivelist{\do\.\do\,\do\:\do\;\do\?\do\!\do\/\do\>\do\-}
\fvset{codes*=\sphinxbreaksattexescapedchars\do\^\^\let\@noligs\nbsphinx@noligs}
Copy link
Contributor

Choose a reason for hiding this comment

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

(comment got lost because I forgot to add it before concluding review)
Most LaTeX packages end with an \endinput on the last line. But it is optional. It gets added automatically when the LaTeX code is extracted from a ".dtx" file containing both LaTeX macros and comments via something called "DocStrip".

@mgeier
Copy link
Member Author

mgeier commented Feb 8, 2023

Thanks as always @jfbu, you are the best!

that LaTeX packages have a "v" before the version number seems to be a community habit

I don't really care about the version number, I've just seen it written like this in some example (but now that you mention it, I think I have also seen it without the "v", and maybe even without a version number?).

So what should I do?

Should I remove the "v"?
Should I remove the whole version number?

I already opted for not keeping it synchronized with the main version number, because I think this would be too much maintenance work.

I don't need the version number, and if LaTeX doesn't need it either, I might as well remove it?

Whenever the file changes in the future it should be enough to update the date, right?

@jfbu
Copy link
Contributor

jfbu commented Feb 8, 2023

Whenever the file changes in the future it should be enough to update the date, right?

Yes, only the date is needed as a general demand on LaTeX package authors. This is to allow users, or rather authors of other packages to require your package at some minimal date, presumably to be sure it will be at a version containing such or such feature.

But the complete truth is that you may completely drop the square brackets and stuff after your \ProvidesPackage. LaTeX will accept ̀\ProvidesPackage{nbsphinx}.

Your nbsphinx.sty is not a priori destined to be downloadable from CTAN as an independent package that arbitrary LaTeX users will use in their documents. Only the elite people using Sphinx will have it (and most of the time will remain blissfully unaware) in their make latex build repertory.

File nbsphinx.sty:

\ProvidesPackage{nbsphinx}

% nothing here

\endinput

file test.tex:

\documentclass{article}

\usepackage{nbsphinx}

\listfiles

\begin{document}
Nothing
\end{document}

Then execute pdflatex test. Nothing bad happens and console output displays (and log contains):

*File List*
 article.cls    2022/07/02 v1.4n Standard LaTeX document class
  size10.clo    2022/07/02 v1.4n Standard LaTeX file (size option)
nbsphinx.sty    
l3backend-pdftex.def    2023-01-16 L3 backend support: PDF output (pdfTeX)
 ***********

which is a bit silent and mysterious but ok. So for minimizing maintenance you may either do as above or only include a short description and not even insert a date.

If I deliberately use \usepackage{nbsphinx}[2023/01/01] in my test.tex I get only a warning

LaTeX Warning: You have requested, on input line 3, version
               `2023/01/01' of package nbsphinx,
               but only version
               `'
               is available.

but it is not Sphinx who will add a date requirement.

Perhaps some other extension builds in the future upon nbsphinx LaTeX code and they need some feature you added at some date... but then they may as well require nbsphinx itself at some version via their pip requirements so...

@jfbu
Copy link
Contributor

jfbu commented Feb 8, 2023

I already opted for not keeping it synchronized with the main version number, because I think this would be too much maintenance work.

I can only confirm. At Sphinx it is really a pain simply to worry about ̀ sphinx.sty and there are quite a few other .sty files (for most I used ̀\ProvidesFile and then \input from sphinx.sty however I should perhaps have used \ProvidesPackage and \RequirePackage for a bit neater log files ; but I am probably the sole individual on this Earth reading the log files from Sphinx LaTeX builds... )

@mgeier
Copy link
Member Author

mgeier commented Feb 10, 2023

Thanks for the detailed analysis!

I have removed the version number but opted for keeping the date and short description for your log-file-reading convenience.

I am probably the sole individual on this Earth reading the log files from Sphinx LaTeX builds... )

That seems very likely.

@mgeier mgeier merged commit 017d026 into spatialaudio:master Feb 11, 2023
@mgeier mgeier deleted the nbsphinx-sty branch February 11, 2023 14:27
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