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

[DOC] Allow Sphinx PDF build to reflow long decimal expansions in code-blocks to avoid them extending beyond page limits #39325

Merged
merged 3 commits into from
Jan 18, 2025

Conversation

jfbu
Copy link
Contributor

@jfbu jfbu commented Jan 14, 2025

Update conf.py's for A Tour of Sage in PDF format to fix long decimal expansions.

Before:

Capture d’écran 2025-01-14 à 12 28 46

Capture d’écran 2025-01-14 à 12 27 29

After:

Capture d’écran 2025-01-14 à 12 09 05

Additional notes: EDIT The below is partially obsolete. The change has been moved to sage_docbuld/conf.py following #39325 (comment)

  • I took the "before" screenshot from official PDF and the "after" from building locally on a checkout of develop branch, which may explain a number of differences which are not related to the PR at all,
  • the PR only has an impact on wrapping lengthy lines in code-blocks,
  • To build the PDFs locally, I copied over the conf.py, index.rst and image files to a fresh sphinx empty project with some adaptations to conf.py to avoid importing Sage modules which I did not install in my environment, because I feared the whole thing might be a very long process and I am not familiar with Sage and Sage repository. I simply forked it to be able to submit this PR.
  • I removed from the French language conf.py some stuff about \at LaTeX control sequence needing to be undefined to avoid conflict with babel-french, because I am not aware of Sphinx defining any \at at all in the preamble it creates in output .tex file, I suspected this is long obsolete, there is no \at definition to be found in any of Sphinx LaTeX support files and I have never encountered such an issue in my practice of Sphinx with French language.
  • There is also some special configuration in the Italian conf.py which I simply copied over, not deleting. Its original syntax extends latex_elements['preamble'] but except if there is some import during the build of some other conf.py, which appears not to be the case, a direct assignment as done in PR should be equivalent.
  • Tests of the PDF builds were done using latest Sphinx (8.1.3 and dev HEAD).
  • As explained above the PDF builds were done manually and not via the Sage build process which I feared might require me to install a bunch of additional dependencies, etc... which I did not do. So please double-check the effect on a genuine documentation build.

The PR only affects the various A Tour of Sage documents but may perhaps be profitably extended to more Sage documentation in future.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@kwankyu
Copy link
Collaborator

kwankyu commented Jan 15, 2025

The PR only affects the various A Tour of Sage documents but may perhaps be profitably extended to more Sage documentation in future.

Why not put 'sphinxsetup': 'verbatimforcewraps=true' into src/sage_docbuild/conf.py (line 610)?

@jfbu
Copy link
Contributor Author

jfbu commented Jan 15, 2025

The PR only affects the various A Tour of Sage documents but may perhaps be profitably extended to more Sage documentation in future.

Why not put 'sphinxsetup': 'verbatimforcewraps=true' into src/sage_docbuild/conf.py (line 610)?

Ah sorry I overlooked completely the existence of this conf.py and I should have paid some minimal attention to the lines

from sage_docbuild.conf import release, latex_elements

in the various src/doc/**/conf.py. So indeed it seems the correct location is in the master conf.py. (and I did something wrong in the Italian conf.py for a_tour_of_sage).

I knew I was going something wrong, but I really did not have the time to famiiarize myself with the configuration here. I will update my PR later today, can't now.

Copy link

github-actions bot commented Jan 15, 2025

Documentation preview for this PR (built with commit 4e0e9a8; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@jfbu jfbu changed the title [DOC] src/doc/**/conf.py: let a_tour_of_sage.pdf wrap long decimal expansions [DOC] Allow Sphinx PDF build to reflow long decimal expansions in code-blocks to avoid them extending beyond page limits Jan 15, 2025
@jfbu
Copy link
Contributor Author

jfbu commented Jan 15, 2025

The PR only affects the various A Tour of Sage documents but may perhaps be profitably extended to more Sage documentation in future.

Why not put 'sphinxsetup': 'verbatimforcewraps=true' into src/sage_docbuild/conf.py (line 610)?

@kwankyu Done, thanks.

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

Thanks. I agree that this makes the pdf docs look nicer.

@vbraun vbraun merged commit f1b71f2 into sagemath:develop Jan 18, 2025
21 of 23 checks passed
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 19, 2025
…latex work-around

    
This work-around was needed with Pygments<1.0 hence has been obsolete
for more than fifteen years.

Here is the [relevant commit](https://github.com/pygments/pygments/commi
t/14b9433c05086beb67aca7436a96d1e685787d93) from Pygments:
```text
commit 14b9433c05086beb67aca7436a96d1e685787d93
Author: Georg Brandl <[email protected]>
Date:   Fri Sep 12 00:28:08 2008 +0200

    Don't define \at, \lb, \rb as escapes, but prepend commandprefix
here too.
```

Update: it is possible early-history Sphinx had some hardcoded things
nowadays imported dynamically from Pygments such as  the contents of the
file `sphinxhighlight.sty` one finds in the latex build directory (which
does currently `\def\PYGZat{@}`).  Indeed I also found a [similar commit
at Sphinx](https://github.com/sphinx-
doc/sphinx/commit/46544e4986fd2dbdf11d79897c18ef6623956db6),
```text
commit 46544e4986fd2dbdf11d79897c18ef6623956db6
Author: Georg Brandl <[email protected]>
Date:   Thu Sep 11 22:29:35 2008 +0000

    Use a prefix to \at, \lb and \rb since they are probably often used
command names.

```
which was part of [Sphinx 0.5 release](https://github.com/sphinx-
doc/sphinx/releases/tag/v0.5) from November 2008. It is  not fully clear
to me why sagemath#8183 was raised in February 2010, but anyway I am very
confident the issue does not exist at all if producing the PDF docs with
a reasonable version of Sphinx.

There is another conf.py with this work-around but sagemath#39325 already
handles it.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39326
Reported by: Jean-François B.
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 20, 2025
…latex work-around

    
This work-around was needed with Pygments<1.0 hence has been obsolete
for more than fifteen years.

Here is the [relevant commit](https://github.com/pygments/pygments/commi
t/14b9433c05086beb67aca7436a96d1e685787d93) from Pygments:
```text
commit 14b9433c05086beb67aca7436a96d1e685787d93
Author: Georg Brandl <[email protected]>
Date:   Fri Sep 12 00:28:08 2008 +0200

    Don't define \at, \lb, \rb as escapes, but prepend commandprefix
here too.
```

Update: it is possible early-history Sphinx had some hardcoded things
nowadays imported dynamically from Pygments such as  the contents of the
file `sphinxhighlight.sty` one finds in the latex build directory (which
does currently `\def\PYGZat{@}`).  Indeed I also found a [similar commit
at Sphinx](https://github.com/sphinx-
doc/sphinx/commit/46544e4986fd2dbdf11d79897c18ef6623956db6),
```text
commit 46544e4986fd2dbdf11d79897c18ef6623956db6
Author: Georg Brandl <[email protected]>
Date:   Thu Sep 11 22:29:35 2008 +0000

    Use a prefix to \at, \lb and \rb since they are probably often used
command names.

```
which was part of [Sphinx 0.5 release](https://github.com/sphinx-
doc/sphinx/releases/tag/v0.5) from November 2008. It is  not fully clear
to me why sagemath#8183 was raised in February 2010, but anyway I am very
confident the issue does not exist at all if producing the PDF docs with
a reasonable version of Sphinx.

There is another conf.py with this work-around but sagemath#39325 already
handles it.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39326
Reported by: Jean-François B.
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 23, 2025
…latex work-around

    
This work-around was needed with Pygments<1.0 hence has been obsolete
for more than fifteen years.

Here is the [relevant commit](https://github.com/pygments/pygments/commi
t/14b9433c05086beb67aca7436a96d1e685787d93) from Pygments:
```text
commit 14b9433c05086beb67aca7436a96d1e685787d93
Author: Georg Brandl <[email protected]>
Date:   Fri Sep 12 00:28:08 2008 +0200

    Don't define \at, \lb, \rb as escapes, but prepend commandprefix
here too.
```

Update: it is possible early-history Sphinx had some hardcoded things
nowadays imported dynamically from Pygments such as  the contents of the
file `sphinxhighlight.sty` one finds in the latex build directory (which
does currently `\def\PYGZat{@}`).  Indeed I also found a [similar commit
at Sphinx](https://github.com/sphinx-
doc/sphinx/commit/46544e4986fd2dbdf11d79897c18ef6623956db6),
```text
commit 46544e4986fd2dbdf11d79897c18ef6623956db6
Author: Georg Brandl <[email protected]>
Date:   Thu Sep 11 22:29:35 2008 +0000

    Use a prefix to \at, \lb and \rb since they are probably often used
command names.

```
which was part of [Sphinx 0.5 release](https://github.com/sphinx-
doc/sphinx/releases/tag/v0.5) from November 2008. It is  not fully clear
to me why sagemath#8183 was raised in February 2010, but anyway I am very
confident the issue does not exist at all if producing the PDF docs with
a reasonable version of Sphinx.

There is another conf.py with this work-around but sagemath#39325 already
handles it.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39326
Reported by: Jean-François B.
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 25, 2025
…latex work-around

    
This work-around was needed with Pygments<1.0 hence has been obsolete
for more than fifteen years.

Here is the [relevant commit](https://github.com/pygments/pygments/commi
t/14b9433c05086beb67aca7436a96d1e685787d93) from Pygments:
```text
commit 14b9433c05086beb67aca7436a96d1e685787d93
Author: Georg Brandl <[email protected]>
Date:   Fri Sep 12 00:28:08 2008 +0200

    Don't define \at, \lb, \rb as escapes, but prepend commandprefix
here too.
```

Update: it is possible early-history Sphinx had some hardcoded things
nowadays imported dynamically from Pygments such as  the contents of the
file `sphinxhighlight.sty` one finds in the latex build directory (which
does currently `\def\PYGZat{@}`).  Indeed I also found a [similar commit
at Sphinx](https://github.com/sphinx-
doc/sphinx/commit/46544e4986fd2dbdf11d79897c18ef6623956db6),
```text
commit 46544e4986fd2dbdf11d79897c18ef6623956db6
Author: Georg Brandl <[email protected]>
Date:   Thu Sep 11 22:29:35 2008 +0000

    Use a prefix to \at, \lb and \rb since they are probably often used
command names.

```
which was part of [Sphinx 0.5 release](https://github.com/sphinx-
doc/sphinx/releases/tag/v0.5) from November 2008. It is  not fully clear
to me why sagemath#8183 was raised in February 2010, but anyway I am very
confident the issue does not exist at all if producing the PDF docs with
a reasonable version of Sphinx.

There is another conf.py with this work-around but sagemath#39325 already
handles it.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39326
Reported by: Jean-François B.
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 26, 2025
…latex work-around

    
This work-around was needed with Pygments<1.0 hence has been obsolete
for more than fifteen years.

Here is the [relevant commit](https://github.com/pygments/pygments/commi
t/14b9433c05086beb67aca7436a96d1e685787d93) from Pygments:
```text
commit 14b9433c05086beb67aca7436a96d1e685787d93
Author: Georg Brandl <[email protected]>
Date:   Fri Sep 12 00:28:08 2008 +0200

    Don't define \at, \lb, \rb as escapes, but prepend commandprefix
here too.
```

Update: it is possible early-history Sphinx had some hardcoded things
nowadays imported dynamically from Pygments such as  the contents of the
file `sphinxhighlight.sty` one finds in the latex build directory (which
does currently `\def\PYGZat{@}`).  Indeed I also found a [similar commit
at Sphinx](https://github.com/sphinx-
doc/sphinx/commit/46544e4986fd2dbdf11d79897c18ef6623956db6),
```text
commit 46544e4986fd2dbdf11d79897c18ef6623956db6
Author: Georg Brandl <[email protected]>
Date:   Thu Sep 11 22:29:35 2008 +0000

    Use a prefix to \at, \lb and \rb since they are probably often used
command names.

```
which was part of [Sphinx 0.5 release](https://github.com/sphinx-
doc/sphinx/releases/tag/v0.5) from November 2008. It is  not fully clear
to me why sagemath#8183 was raised in February 2010, but anyway I am very
confident the issue does not exist at all if producing the PDF docs with
a reasonable version of Sphinx.

There is another conf.py with this work-around but sagemath#39325 already
handles it.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39326
Reported by: Jean-François B.
Reviewer(s): Kwankyu Lee
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