LaTeX: add styling to general index, similar to domain indices #5583

jfbu · 2018-10-31T18:40:31Z

Closes: #5579. Also closes #5576.

With this here is for example how CPython 3.7 library.pdf renders:

and

Compare to the images posted at #5579.

About the underscore character, it shows fine because CPython uses xelatex for pdf builds and this means another font is used than the Sphinx default Courier. Unfortunately the Courier font used by default for monospace font by Sphinx has an underscore glyph which creates continuous lines... one can see that in Sphinx own pdf doc, see top left.

Sphinx own doc uses \footnotesize\raggedright which explains smaller size.

It fixes #5576 in a way different from #5577: the regex argument of visit_index() in latex.py is used for something completely different: separate from the index a parenthesized comment ("extra") and style it especially as in Python Module Index.

Another slightly breaking change is in \sphinxstyleindexextra, the parentheses stay upright. I feel it looks better. (I will do more detailed review some time later)

Closes: sphinx-doc#5579. Also closes sphinx-doc#5576.

codecov · 2018-10-31T20:51:59Z

Codecov Report

Merging #5583 into 1.8 will increase coverage by <.01%.
The diff coverage is 93.93%.

@@            Coverage Diff             @@
##              1.8    #5583      +/-   ##
==========================================
+ Coverage   82.05%   82.06%   +<.01%     
==========================================
  Files         306      306              
  Lines       40390    40408      +18     
  Branches     6241     6244       +3     
==========================================
+ Hits        33144    33159      +15     
- Misses       5862     5863       +1     
- Partials     1384     1386       +2

Impacted Files	Coverage Δ
tests/test_build_latex.py	`95.27% <100%> (ø)`	⬆️
sphinx/writers/latex.py	`84.12% <92%> (+0.05%)`	⬆️
sphinx/builders/html.py	`82.83% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47bd24e...00a75b2. Read the comment docs.

jfbu · 2018-11-01T09:24:42Z

sphinx/texinputs/sphinx.sty

@@ -1623,8 +1623,11 @@

 % additional customizable styling
 \def\sphinxstyleindexentry   #1{\texttt{#1}}
-\def\sphinxstyleindexextra   #1{ \emph{(#1)}}
+\def\sphinxstyleindexextra   #1{ (\emph{#1})}


I moved the parentheses outside the scope of \emph. imho, it looks better this way.

jfbu · 2018-11-01T09:27:42Z

sphinx/texinputs/sphinx.sty

 \def\sphinxstyleindexpageref #1{, \pageref{#1}}
+\def\sphinxstyleindexpagemain#1{\textbf{#1}}
+\protected\def\spxentry#1{#1}% will get \let to \sphinxstyleindexentry in index
+\protected\def\spxextra#1{#1}% will get \let to \sphinxstyleindexextra in index


The visit_index() from LaTeX writer will use \spxentry and \spxextra, this makes smaller LaTeX file than using everywhere \sphinxstyleindexentry, \sphinxstyleindexextra. The macros need to be \protected because they are used inside \index{} macro, and this may occur for example in a tabulary environment (same reason why verbatim can't be used inside tabulary).

I decided to re-use the same \sphinxstyleindexentry and \sphinxstyleindexextra already used by LaTeX writer for domain indices at

sphinx/sphinx/writers/latex.py

Line 784 in 8b687ac

def generate_indices(self):

.

An alternative would be to define independent macros \sphinxstylegeneralindexentry, etc...

The \spxentry/\spxextra are privately used, only \sphinxstyleindexentry/... should get redefined by user. The reason for \spxentry/\spxextra is for shorter macro names, hence smaller file.

On the xindy side I looked a bit about using markup-indexentry which currently (in sphinx.xdy) is

(markup-indexentry :open "~n \item " :depth 0)

but did not see a way to add there the \spxentry{..} wrapper. Because in case of sub-items, a closing } for example would enclose all sub-items, causing errors. And anyway, the @\spxentry{...} works both with makeindex and xindy. Furthermore I later decided (to match the domain indices) to use \spxentry{...}\spxextra{...}, not \spxentry{...\spxextra{...}}, which I had started with.

jfbu · 2018-11-01T09:32:25Z

sphinx/texinputs/sphinx.xdy

 (markup-locref :open "\textbf{\hyperpage{" :close "}}" :attr "textbf")
 (markup-locref :open "\textit{\hyperpage{" :close "}}" :attr "textit")
 (markup-locref :open "\emph{\hyperpage{" :close "}}" :attr "emph")
+(markup-locref :open "\spxpagem{\hyperpage{" :close "}}" :attr "spxpagem")
 (markup-locref :open "\hyperpage{" :close "}" :attr "default")


Initially I had also added a spxpage attribute so that one would would have \spxpage{\hyperpage{37}} for example, allowing to style the page number. But this would mean all \index macros would end in \index{...|spxpage] increasing size and overhead of LaTeX file. Then I thought about simply modifying the "default" above to do ...\spxpage{\hyperpage{..., but this method would have been xindy-only. Only the \index{....|spxpage} would have also affected makeindex way.

In the end I dropped this: it is always possible if wanted to style the index page number at LaTeX macro level by modifying \hyperpage.

jfbu · 2018-11-01T09:40:41Z

sphinx/texinputs/sphinx.xdy

@@ -193,6 +194,9 @@
 (markup-index :open  "\begin{sphinxtheindex}
 \let\lettergroup\sphinxstyleindexlettergroup
 \let\lettergroupDefault\sphinxstyleindexlettergroupDefault
+\let\spxpagem\sphinxstyleindexpagemain
+\let\spxentry\sphinxstyleindexentry
+\let\spxextra\sphinxstyleindexextra


Regarding \sphinxstyleindexentry and \sphinxstyleindexextra they are the macros used by the LaTeX writer for the mark-up associated with domain indices. The method above allows same mark-up also in general index. But it means too that separate customization will mean from user some extra measure. One method would add to preamble some definition of \stylegeneralindexentry, \stylegeneralindexextra macros, and insert \let\sphinxstyleindexentry\stylegeneralindexentry\let\... before the final index via the latex_elements 'printindex' key.

I also hesitated about moving these \let to the definition of the sphinxtheindex environment, but the latter is used for all indices, including domain indices, where \spxentry and \spxextra do not arise.

jfbu · 2018-11-01T09:59:05Z

sphinx/writers/latex.py

@@ -1884,7 +1884,7 @@ def depart_attribution(self, node):
        # type: (nodes.Node) -> None
        self.body.append('\n\\end{flushright}\n')

-    def visit_index(self, node, scre=re.compile(r';\s*')):
+    def visit_index(self, node, extrare=re.compile(r'^(.*\S)\s+\(([^()]*)\)\s*$')):


This is breaking change for extensions who counted on the third argument of visit_index to be the regex allowing to recognize semi-colon. A priori this was not used as public API? so I felt there is no problem using this for something completely different.

The aim of the regex is to identify strings of the type "text (extra in parentheses)". Then the extra in parenthesis will be styled especially. This is a new feature which has drawback that sometimes the parentheses were used by Sphinx user with a somewhat different intent. I found an example in Sphinx own docs, at line 125 of doc/man/sphinx-quickstart.rst:

.. option:: --use-make-mode (-m), --no-use-make-mode (-M)

In this case this will produce in LaTeX file:

\index{--use-make-mode (-m), --no-use-make-mode (-M)@\spxentry{--use-make-mode (-m), --no-use-make-mode}\spxextra{-M}!...}

which causes the -M to be emphasized. In general any

.. option:: --long-name (-l)

will get the -l styled by \spxextra, where the --long-name gets styled by \spxentry.

But this syntax is not the one described at https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#directive-option, so maybe this is not really breaking change. And it only affects typography.

Yes, I agree with this is not a public API. But this must be a breaking change. So -1 for changing signature of method.

I think marking scre keyword as deprecated is better. And I think extrare is not needed to substitute. So how about moving it to constant?

EXTRA_RE = re.compile('...') ... def visit_index(self, node, scre=None): if scre: warnings.warn('scre option for visit_index() is deprecated') ...

jfbu · 2018-11-01T10:05:33Z

sphinx/writers/latex.py

+        def style(string):
+            match = extrare.match(string)
+            if match:
+                return match.expand(r'\\spxentry{\1}\\spxextra{\2}')


This mark-up is modeled on

sphinx/sphinx/writers/latex.py

Lines 800 to 804 in 8b687ac

if entry[4]:

# add "extra" info

ret.append('\\sphinxstyleindexextra{%s}' % self.encode(entry[4]))

ret.append('\\sphinxstyleindexpageref{%s:%s}\n' %

(entry[2], self.idescape(entry[3])))

which applies to entries for domain indices. But here the "extra" is recovered from this input syntax:

.. index:: single: foo (bar)

jfbu · 2018-11-01T10:06:24Z

sphinx/writers/latex.py

+                    except ValueError:
+                        p = escape(split_into(1, 'single', string)[0])
+                        P = style(p)
+                        self.body.append(r'\index{%s@%s%s}' % (p, P, m))


This also fixes #5576.

jfbu · 2018-11-01T10:09:20Z

sphinx/writers/latex.py

+                        r'\index{%s@\spxentry{%s}!%s, %s@\spxentry{%s, %s}%s}'
+                        r'\index{%s@\spxentry{%s}!%s %s@\spxentry{%s %s}%s}' %
+                        (p1, p1, p2, p3, p2, p3, m, p2, p2, p3, p1, p3, p1, m,
+                         p3, p3, p1, p2, p1, p2, m))


Here for triples I dropped addition of \spxextra styling. But I should add it, I guess.

I will push update to incorporate \spxextra styling for p1, p2, p3 if they use foo (bar) style also in this "triple" case. (done at 0f45cf7)

jfbu · 2018-11-01T10:10:30Z

sphinx/writers/latex.py

                elif type == 'see':
                    p1, p2 = [escape(x) for x in split_into(2, 'see', string)]
-                    self.body.append(r'\index{%s|see{%s}}' % (p1, p2))
+                    P1 = style(p1)
+                    self.body.append(r'\index{%s@%s|see{%s}}' % (p1, P1, p2))


I did not make any attempt into adding mark-up in the see{%s} part as I expected problems, especially to maintain compatibility with both makeindex and xindy.

tk0miya

LGTM with nits

tk0miya · 2018-11-03T16:19:33Z

sphinx/writers/latex.py

@@ -1884,7 +1884,7 @@ def depart_attribution(self, node):
        # type: (nodes.Node) -> None
        self.body.append('\n\\end{flushright}\n')

-    def visit_index(self, node, scre=re.compile(r';\s*')):
+    def visit_index(self, node, extrare=re.compile(r'^(.*\S)\s+\(([^()]*)\)\s*$')):


Yes, I agree with this is not a public API. But this must be a breaking change. So -1 for changing signature of method.

I think marking scre keyword as deprecated is better. And I think extrare is not needed to substitute. So how about moving it to constant?

EXTRA_RE = re.compile('...') ... def visit_index(self, node, scre=None): if scre: warnings.warn('scre option for visit_index() is deprecated') ...

And it is already ignored.

tk0miya

LGTM!

jfbu · 2018-11-06T17:12:07Z

@tk0miya thanks for quick reviewing! I will merge now into 1.8.2 although this is less minimal change than #5577. My estimate indeed is that absence of styling of general index in LaTeX output was a bug.

LaTeX: add styling to general index, similar to domain indices

4df3b06

Closes: sphinx-doc#5579. Also closes sphinx-doc#5576.

jfbu added type:bug builder:latex labels Oct 31, 2018

jfbu added this to the 1.8.2 milestone Oct 31, 2018

jfbu commented Nov 1, 2018

View reviewed changes

LaTeX general index: also style "extras" in triple entries

0f45cf7

jfbu mentioned this pull request Nov 1, 2018

make all-pdf fails invocation of xindy #5561

Closed

tk0miya requested changes Nov 3, 2018

View reviewed changes

LaTeX: deprecate optional argument scre to visit_index()

00a75b2

And it is already ignored.

tk0miya approved these changes Nov 6, 2018

View reviewed changes

jfbu merged commit c9999b1 into sphinx-doc:1.8 Nov 6, 2018

This was referenced Nov 6, 2018

LaTeX: use lookbehind regex to not match semicolon at start #5577

Closed

latex writer incorrectly handles indexing a sole semi-colon character #5576

Closed

LaTeX: entries in general index are not styled like entries of domain indices #5579

Closed

jfbu deleted the 5579_style_index_entries branch November 12, 2018 08:29

github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LaTeX: add styling to general index, similar to domain indices #5583

LaTeX: add styling to general index, similar to domain indices #5583

jfbu commented Oct 31, 2018

codecov bot commented Oct 31, 2018 •

edited

jfbu Nov 1, 2018

jfbu Nov 1, 2018

jfbu Nov 1, 2018

jfbu Nov 1, 2018

jfbu Nov 1, 2018

jfbu Nov 1, 2018

tk0miya Nov 3, 2018

jfbu Nov 1, 2018

jfbu Nov 1, 2018

jfbu Nov 1, 2018

jfbu Nov 1, 2018 •

edited

jfbu Nov 1, 2018

tk0miya left a comment

tk0miya Nov 3, 2018

tk0miya left a comment

jfbu commented Nov 6, 2018

	if entry[4]:
	# add "extra" info
	ret.append('\\sphinxstyleindexextra{%s}' % self.encode(entry[4]))
	ret.append('\\sphinxstyleindexpageref{%s:%s}\n' %
	(entry[2], self.idescape(entry[3])))

LaTeX: add styling to general index, similar to domain indices #5583

LaTeX: add styling to general index, similar to domain indices #5583

Conversation

jfbu commented Oct 31, 2018

codecov bot commented Oct 31, 2018 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfbu Nov 1, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tk0miya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tk0miya left a comment

Choose a reason for hiding this comment

jfbu commented Nov 6, 2018

codecov bot commented Oct 31, 2018 •

edited

jfbu Nov 1, 2018 •

edited