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: fix vertical spacing for cpp:function. #10087

Merged
merged 3 commits into from Feb 14, 2022

Conversation

marxin
Copy link
Contributor

@marxin marxin commented Jan 11, 2022

s described in the issue, there is a pair of
\pysigstartmultiline/\pysigstopmultiline for each desc_signature
element and thus there is extra spacing when multiple functions
are documented.

With the patch applied, I can see the following rendering:
Screenshot from 2022-01-11 15-24-13

Fixes: #9924.

As described in the issue, there is a pair of
`\pysigstartmultiline/\pysigstopmultiline` for each `desc_signature`
element and thus there is extra spacing when multiple functions
are documented.

Fixes: sphinx-doc#9924.
@tk0miya tk0miya requested a review from jfbu January 11, 2022 17:48
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.

Thanks! I since re-read #9924 (comment) with much more attention, comparing what your PR does to LaTeX output with current Sphinx, and I sense I now understand better the matter. I see how my remark "this issue would go away from having a single pair of \pysigstartmultiline/\pysigstopmultiline" is actually what you achieve. But my understanding is that what is needed is to let your PR enrich, not replace actual situation. Your PR recycles existing \pysigstopmultiline mark-up but if we are to make possible this @jakobandersen suggestion:

So I guess the spacing should be:

- between `<desc>` nodes: moderate vertical gap
- between `<desc_signature>` nodes (no matter `is_multiline`): a bit of extra vertical gap
- between `<desc_signature_line>` nodes: no vertical gap, i.e., single line spacing

what seems to be needed is to keep former \pysigstartmultiline/\pysigstopmultiline mark-up and add another one \pysigstartsignatures/\pysigstopsignatures at higher level. I understand there is a stumbling block here as one needs to provide some LaTeX definitions...

My intention is thus to push to your branch some commit doing that once I get the time. Not sure now if in the end I will be content with same pdf output as from your proposal (i.e. single line spacing throughout multiple signatures sharing same content) or try to get some sort of better granularity as suggested by @jakobandersen. But at any rate the LaTeX placeholder mark-up at least will be in place allowing to achieve the control which is needed for that.

@marxin
Copy link
Contributor Author

marxin commented Jan 12, 2022

My intention is thus to push to your branch some commit doing that once I get the time. Not sure now if in the end I will be content with same pdf output as from your proposal (i.e. single line spacing throughout multiple signatures sharing same content) or try to get some sort of better granularity as suggested by @jakobandersen. But at any rate the LaTeX placeholder mark-up at least will be in place allowing to achieve the control which is needed for that.

To be honest my pull request was intended as a starting point for a discussion. I would appreciate your help and I think the addition of \pysigstartsignatures/\pysigstopsignatures (with proper vertical spacing) is the right way to go. Can you please do that once you have some spare cycles? Thanks.

@jfbu
Copy link
Contributor

jfbu commented Jan 13, 2022

I will soon push a commit to implement above comments. Simply mentioning here that I became aware that the merge of my #9941 into 4.x has broken the effect of my earlier #9946 (related to #9926). This infelicity will be fixed by to-be-pushed commit here.

This fixes sphinx-doc#9924

Removes sphinx-doc#9941 complex LaTeX macros, which incidentally had broken
effect of sphinx-doc#9946 on issue sphinx-doc#9926, as they become unneeded after the
refactoring of \pysigline/\pysiglinewithargs expansion context.
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.

I have experimented with this source:

.. py:function:: __dpd_addsd3 (a, b)
                 __bid_addsd3 (a, b)
                 __dpd_adddd3 (a, b)
                 __bid_adddd3 (a, b)
                 __dpd_addtd3 (a, b)
                 __bid_addtd3 (a, b)

    something

.. cpp:function:: _Decimal32 __dpd_addsd3 (_Decimal32 a, _Decimal32 b)
                  _Decimal32 __bid_addsd3 (_Decimal32 a, _Decimal32 b)
                  _Decimal64 __dpd_adddd3 (_Decimal64 a, _Decimal64 b)
                  _Decimal64 __bid_adddd3 (_Decimal64 a, _Decimal64 b)
                  _Decimal128 __dpd_addtd3 (_Decimal128 a, _Decimal128 b)
                  _Decimal128 __bid_addtd3 (_Decimal128 a, _Decimal128 b)

    something

.. cpp:function:: template<typename T> \
                  _Decimal32 __dpd_addsd3 (_Decimal32 a, _Decimal32 b)
                  template<typename T> \
                  _Decimal32 __bid_addsd3 (_Decimal32 a, _Decimal32 b)
                  template<typename T> \
                  _Decimal64 __dpd_adddd3 (_Decimal64 a, _Decimal64 b)
                  template<typename T> \
                  _Decimal64 __bid_adddd3 (_Decimal64 a, _Decimal64 b)
                  template<typename T> \
                  _Decimal128 __dpd_addtd3 (_Decimal128 a, _Decimal128 b)
                  template<typename T> \
                  _Decimal128 __bid_addtd3 (_Decimal128 a, _Decimal128 b)

    something

.. option:: -mmmx
            -msse
            -msse2

    something

and obtained this:
Capture d’écran 2022-01-13 à 19 21 21

sphinx/texinputs/sphinxlatexobjects.sty Show resolved Hide resolved
@jfbu
Copy link
Contributor

jfbu commented Jan 13, 2022

CI test failure seems unrelated to this PR. The failed tests pass at my locale

platform darwin -- Python 3.8.7, pytest-6.2.2, py-1.10.0, pluggy-0.13.1 -- /..../.venv38/bin/python3
cachedir: .pytest_cache
libraries: Sphinx-4.4.0+/63654129c, docutils-0.16

@jakobandersen
Copy link
Contributor

Generally it looks good, though for the multi-line declarations (the function templates) the gap between each of them feels a bit too large compare to the gap between the last one and then the description.

@jfbu
Copy link
Contributor

jfbu commented Jan 14, 2022

Generally it looks good, though for the multi-line declarations (the function templates) the gap between each of them feels a bit too large compare to the gap between the last one and then the description.

thanks @jakobandersen can you check now? I have ensured exact same gap from last signature to description as for between signatures. Should I add a separate parameter for that?

btw, achieving this required some strange LaTeX incantations, because we are pushing here the legacy logic of rendering signatures via \item far beyond what LaTeX considered as use-cases for \list/\item, and I try to not hack into LaTeX internals but only employ "user-level" interface. But it seems to work as I could check via compiling our own sphinx.pdf and checking-out the rendering, in particular in the "Domains" section.

another remark is that it would be quite complicated to try to put into place the small extra vertical separation between signatures only in case one of them is multiline. So the 3pt extra space which allows to separate from one another multi-line signatures applies throughout to all cases of multiple signatures for common description.

@jakobandersen
Copy link
Contributor

@jfbu, thanks for Latex haxing :-). Do you happen to have a screen shot similar to the last one?

@jfbu
Copy link
Contributor

jfbu commented Jan 15, 2022

@jakobandersen sure here we go with some from a build of our own doc:
Capture d’écran 2022-01-15 à 15 47 09
Capture d’écran 2022-01-15 à 15 49 14
Capture d’écran 2022-01-15 à 15 49 40
Capture d’écran 2022-01-15 à 15 55 06

% signature ending up separated from description (due to voodoo next)
\penalty-100
% 2) some voodoo to separate last signature from description in a manner
% robust with respect to the latter being itself a LaTeX list object
Copy link
Contributor

Choose a reason for hiding this comment

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

"being itself possibly a LaTeX list object"

@jakobandersen
Copy link
Contributor

Thanks! Too me it looks good, though I rarely use the Latex output my self so if someone who uses it more, have a strong opinion then please overrule :-).

@marxin
Copy link
Contributor Author

marxin commented Feb 3, 2022

May I please ping this pull request?

@jfbu jfbu merged commit 6768577 into sphinx-doc:4.x Feb 14, 2022
@jfbu
Copy link
Contributor

jfbu commented Feb 14, 2022

Sorry for late, I had forgotten this (as I reboot every two weeks) and viewing #10158 I was perplexed for a long time that I had a dim memory the sphinxlatexobjects.sty code I was looking at on 4.x branch was obsolete, but I was searching my own PRs to no avail for a while until I remembered this one ;-)

jfbu added a commit that referenced this pull request Feb 14, 2022
@jfbu jfbu added this to the 4.5.0 milestone Feb 14, 2022
@marxin
Copy link
Contributor Author

marxin commented Feb 15, 2022

Heh ;) Thanks for merging that and for help you provided.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2022
@marxin marxin deleted the vertical-spaces-for-cpp-functions branch June 17, 2022 11:10
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.

Multi-line .. function:: directive has big vertical spacing in Latexpdf
3 participants