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: add styling to general index, similar to domain indices #5583

Merged
merged 3 commits into from Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions sphinx/texinputs/python.ist
Expand Up @@ -4,6 +4,9 @@ heading_prefix " \\bigletter "

preamble "\\begin{sphinxtheindex}
\\let\\bigletter\\sphinxstyleindexlettergroup
\\let\\spxpagem \\sphinxstyleindexpagemain
\\let\\spxentry \\sphinxstyleindexentry
\\let\\spxextra \\sphinxstyleindexextra

"

Expand Down
5 changes: 4 additions & 1 deletion sphinx/texinputs/sphinx.sty
Expand Up @@ -1623,8 +1623,11 @@

% additional customizable styling
\def\sphinxstyleindexentry #1{\texttt{#1}}
\def\sphinxstyleindexextra #1{ \emph{(#1)}}
\def\sphinxstyleindexextra #1{ (\emph{#1})}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

\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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

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.

\def\sphinxstyleindexlettergroup #1%
{{\Large\sffamily#1}\nopagebreak\vspace{1mm}}
\def\sphinxstyleindexlettergroupDefault #1%
Expand Down
8 changes: 6 additions & 2 deletions sphinx/texinputs/sphinx.xdy
Expand Up @@ -3,11 +3,12 @@
;; Unfortunately xindy is out-of-the-box hyperref-incompatible. This
;; configuration is a workaround, which requires to pass option
;; hyperindex=false to hyperref.
;; textit and emph not currently used by Sphinx LaTeX writer.
(define-attributes (("textbf" "textit" "emph" "default")))
;; textit and emph not currently used, spxpagem replaces former textbf
(define-attributes (("textbf" "textit" "emph" "spxpagem" "default")))
(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")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


(require "numeric-sort.xdy")
Expand Down Expand Up @@ -193,6 +194,9 @@
(markup-index :open "\begin{sphinxtheindex}
\let\lettergroup\sphinxstyleindexlettergroup
\let\lettergroupDefault\sphinxstyleindexlettergroupDefault
\let\spxpagem\sphinxstyleindexpagemain
\let\spxentry\sphinxstyleindexentry
\let\spxextra\sphinxstyleindexextra
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


"
:close "
Expand Down
40 changes: 29 additions & 11 deletions sphinx/writers/latex.py
Expand Up @@ -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*$')):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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')

    ...

# type: (nodes.Node, Pattern) -> None
def escape(value):
value = self.encode(value)
Expand All @@ -1895,33 +1895,51 @@ def escape(value):
value = value.replace('!', '"!')
return value

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

Choose a reason for hiding this comment

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

This mark-up is modeled on

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)

else:
return '\\spxentry{%s}' % string

if not node.get('inline', True):
self.body.append('\n')
entries = node['entries']
for type, string, tid, ismain, key_ in entries:
m = ''
if ismain:
m = '|textbf'
m = '|spxpagem'
try:
if type == 'single':
p = scre.sub('!', escape(string))
self.body.append(r'\index{%s%s}' % (p, m))
try:
p1, p2 = [escape(x) for x in split_into(2, 'single', string)]
P1, P2 = style(p1), style(p2)
self.body.append(r'\index{%s@%s!%s@%s%s}' % (p1, P1, p2, P2, m))
except ValueError:
p = escape(split_into(1, 'single', string)[0])
P = style(p)
self.body.append(r'\index{%s@%s%s}' % (p, P, m))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also fixes #5576.

elif type == 'pair':
p1, p2 = [escape(x) for x in split_into(2, 'pair', string)]
self.body.append(r'\index{%s!%s%s}\index{%s!%s%s}' %
(p1, p2, m, p2, p1, m))
P1, P2 = style(p1), style(p2)
self.body.append(r'\index{%s@%s!%s@%s%s}\index{%s@%s!%s@%s%s}' %
(p1, P1, p2, P2, m, p2, P2, p1, P1, m))
elif type == 'triple':
p1, p2, p3 = [escape(x) for x in split_into(3, 'triple', string)]
self.body.append(
r'\index{%s!%s %s%s}\index{%s!%s, %s%s}'
r'\index{%s!%s %s%s}' %
(p1, p2, p3, m, p2, p3, p1, m, p3, p1, p2, m))
r'\index{%s@\spxentry{%s}!%s %s@\spxentry{%s %s}%s}'
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))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@jfbu jfbu Nov 1, 2018

Choose a reason for hiding this comment

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

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)

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))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

elif type == 'seealso':
p1, p2 = [escape(x) for x in split_into(2, 'seealso', 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))
else:
logger.warning(__('unknown index entry type %s found'), type)
except ValueError as err:
Expand Down
26 changes: 16 additions & 10 deletions tests/test_build_latex.py
Expand Up @@ -1199,9 +1199,13 @@ def test_latex_index(app, status, warning):
app.builder.build_all()

result = (app.outdir / 'Python.tex').text(encoding='utf8')
assert 'A \\index{famous}famous \\index{equation}equation:\n' in result
assert '\n\\index{Einstein}\\index{relativity}\\ignorespaces \nand' in result
assert '\n\\index{main \\sphinxleftcurlybrace{}}\\ignorespaces ' in result
assert ('A \\index{famous@\\spxentry{famous}}famous '
'\\index{equation@\\spxentry{equation}}equation:\n' in result)
assert ('\n\\index{Einstein@\\spxentry{Einstein}}'
'\\index{relativity@\\spxentry{relativity}}'
'\\ignorespaces \nand') in result
assert ('\n\\index{main \\sphinxleftcurlybrace{}@\\spxentry{'
'main \\sphinxleftcurlybrace{}}}\\ignorespaces ' in result)


@pytest.mark.sphinx('latex', testroot='latex-equations')
Expand Down Expand Up @@ -1269,20 +1273,22 @@ def test_latex_glossary(app, status, warning):
app.builder.build_all()

result = (app.outdir / 'test.tex').text(encoding='utf8')
assert (u'\\item[{änhlich\\index{änhlich|textbf}\\phantomsection'
assert (u'\\item[{änhlich\\index{änhlich@\\spxentry{änhlich}|spxpagem}'
r'\phantomsection'
r'\label{\detokenize{index:term-anhlich}}}] \leavevmode' in result)
assert (r'\item[{boson\index{boson|textbf}\phantomsection'
assert (r'\item[{boson\index{boson@\spxentry{boson}|spxpagem}\phantomsection'
r'\label{\detokenize{index:term-boson}}}] \leavevmode' in result)
assert (r'\item[{\sphinxstyleemphasis{fermion}\index{fermion|textbf}'
assert (r'\item[{\sphinxstyleemphasis{fermion}'
r'\index{fermion@\spxentry{fermion}|spxpagem}'
r'\phantomsection'
r'\label{\detokenize{index:term-fermion}}}] \leavevmode' in result)
assert (r'\item[{tauon\index{tauon|textbf}\phantomsection'
assert (r'\item[{tauon\index{tauon@\spxentry{tauon}|spxpagem}\phantomsection'
r'\label{\detokenize{index:term-tauon}}}] \leavevmode'
r'\item[{myon\index{myon|textbf}\phantomsection'
r'\item[{myon\index{myon@\spxentry{myon}|spxpagem}\phantomsection'
r'\label{\detokenize{index:term-myon}}}] \leavevmode'
r'\item[{electron\index{electron|textbf}\phantomsection'
r'\item[{electron\index{electron@\spxentry{electron}|spxpagem}\phantomsection'
r'\label{\detokenize{index:term-electron}}}] \leavevmode' in result)
assert (u'\\item[{über\\index{über|textbf}\\phantomsection'
assert (u'\\item[{über\\index{über@\\spxentry{über}|spxpagem}\\phantomsection'
r'\label{\detokenize{index:term-uber}}}] \leavevmode' in result)


Expand Down