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

Fix #777: (part II) LaTeX output "too deeply nested" #3096

Merged
merged 4 commits into from
Oct 27, 2016

Conversation

jfbu
Copy link
Contributor

@jfbu jfbu commented Oct 25, 2016

The standard classes have a severe cap on maximal nesting of list-like
environments (a total of six levels, and four for each of enumerate and
itemize). This commit defines a new key 'maxnestingdepth'
'maxlistdepth'. Only if it is set, sphinx.sty will do some hack to remove the
LaTeX limitations.

Note: merged PR makes the key silently inoperative if package "enumitem"
is detected, so as to not change anything to projects already using "enumitem"
facilities.

@jfbu
Copy link
Contributor Author

jfbu commented Oct 25, 2016

Tested with this source, allowing deepest level equal to 10 :

(edited because original had an indentation error and in fact went to a quoted block at level 11, which did not raise a LaTeX error on compilation due to an off by one test in original commit, fixed since; also the image has been replaced because the key name was changed to 'maxlistdepth' since original post)

Welcome to temp's documentation!
================================

Nesting lists
-------------

1. a

   1. b

      1. c

         1. d

            1. e

               1. f

                  1. g

                     1. h

                        1. This should work (level 9)

                             and we can even go one level deeper (this is quoted)

                           - with a non-number list

                             .. code-block:: python

                                 def foo(): # and a code block in it

                           - did it work ?

                             we used ``'maxlistdepth' : '10'``


- a

   - b

      - c

         - d

            - e

               - f

                  - g

                     - h

                       1. This should work (level 9)

                             and we can even go one level deeper (this is quoted)

                          1. with a numbered list

                             .. code-block:: python

                                  def foo(): # and a code block in it

                          2. did it work ?

                             we used ``'maxlistdepth' : '10'``

which, with a4paper and 'maxlistdepth' : '10' in latex_elements gives:

temp 5-correct

edit: the 'maxlistdepth' key setting is ignored if the package "enumitem" is detected; in that case use the "enumitem"'s \setlistdepth and allied commands.

@jfbu jfbu added this to the 1.5 milestone Oct 25, 2016
@tk0miya
Copy link
Member

tk0miya commented Oct 26, 2016

Nothing from my side.
I confirmed it works fine :-)

jfbu added 2 commits October 26, 2016 10:30
The standard classes have a severe cap on maximal nesting of list-like
environments (a total of six levels, and four for each of enumerate and
itemize). This commit defines a new key ``'maxlistdepth'``. _Only_ if it
is set (i.e. non-empty) will sphinx.sty do some hack to lift the LaTeX
limitations and extend the maximal list depth to the desired value.
will facilitate adding options in the future, with a "key=value" syntax.
@jfbu jfbu force-pushed the latex_fixtoodeeplynested branch from 48079dd to 4470e7d Compare October 26, 2016 10:11
@jfbu
Copy link
Contributor Author

jfbu commented Oct 26, 2016

thanks @tk0miya! I have squashed all commits and added use of package "kvoptions" (which anyhow is already required by "hyperref"), to facilitate handling of "key=value" options if more are added in the future. This already streamlines my ad hoc handling of maxlistdepth=<N> option in parent commit.

This could help user-styling of LaTeX via for example a single 'passoptionstosphinx' key. For example to set some dimensions, colours, etc...

@jfbu
Copy link
Contributor Author

jfbu commented Oct 26, 2016

I wanted to add a root test, but there is nothing to check in LaTeX output, the test should do at least one pdflatex. And I don't know how to add such a test.

@jfbu jfbu force-pushed the latex_fixtoodeeplynested branch from a7b5efd to 4e2bdf6 Compare October 27, 2016 07:24
@jfbu
Copy link
Contributor Author

jfbu commented Oct 27, 2016

merging now as testing looks ok. Projects using "enumitem" to circumvent the LaTeX default limitations will not be affected by this. New projects may use the novel key as a quick work-around, if needed.

@jfbu jfbu merged commit 5fa47df into sphinx-doc:master Oct 27, 2016
@jfbu jfbu deleted the latex_fixtoodeeplynested branch October 27, 2016 08:30
@tk0miya
Copy link
Member

tk0miya commented Oct 27, 2016

I wanted to add a root test, but there is nothing to check in LaTeX output, the test should do at least one pdflatex. And I don't know how to add such a test.

What check do you want to do? Viewing generated PDF? Checking logs of pdflatex?

@jfbu
Copy link
Contributor Author

jfbu commented Oct 27, 2016

@tk0miya my question was more elementary: simply check that doing pdflatex once works; all the tests I checked in test_build_latex.py seem to be about verifying that the produced latex file contains something. But in the case of the input as in my comment #3096 (comment) and 'maxlistdepth' : '10', we actually need to run pdflatex to be sure no conflict arises in the future if the default configuration has some new package, or if some new domentclass is proposed to user among those already used by Sphinx.

@tk0miya
Copy link
Member

tk0miya commented Oct 27, 2016

The build_latex_doc() invokes pdflatex, lualatex and xelatex to build PDF actually.
https://github.com/sphinx-doc/sphinx/blob/master/tests/test_build_latex.py#L71

I can make a helper function to invoke pdflatex if you want.
But latex compilation is too heavy operation. so we should use it carefully to keep tests quick.

@jfbu
Copy link
Contributor Author

jfbu commented Oct 28, 2016

@tk0miya thanks for explanation I now understand better. I made for experiment a branch https://github.com/jfbu/sphinx/tree/testtest to get make test TEST=test_build_latex.py to do what I wanted. As this is a try, I did it in a doubled manner:

  1. add a test to /root because there the project is automatically run through pdflatex (and the other engines)
  2. add a unit test to /roots but with extra instruction to actually run pdflatex on latex file (at https://github.com/jfbu/sphinx/blob/feadf4346c36a0b5e374bd4d2f7f11d8a382aee4/tests/test_build_latex.py#L700)

This is purely for me learning how it works. I agree we should be careful not to overload tests.

@tk0miya
Copy link
Member

tk0miya commented Nov 4, 2016

I added compile_latex_document() as a helper function at 3f4721e.
It would make the 2nd case simple.

About 1st case, /root is used to common test case for all builders. So please move to individual testcase if you want to test it on each engine.

Personally, I want to reduce the content of /root directory. It makes all tests fast.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2021
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