Opened 7 years ago

Closed 7 years ago

#14382 closed defect (fixed)

Fix LaTeXing of strings

Reported by: novoselt Owned by: was
Priority: major Milestone: sage-5.12
Component: user interface Keywords: latex
Cc: cremona, ddrake, jhpalmieri Merged in: sage-5.12.beta3
Authors: Andrey Novoseltsev Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jhpalmieri)

#11498 made "ASCII-art" strings look reasonably well when displayed via LaTeX methods. One of the changes was damaged in #9774, resulting in invalid LaTeX code that works in MathJax?, but not in SageTeX. Let's fix it again.

Apply trac_14382-texttt.patch.

Attachments (5)

trac_14382_doctests.2.patch (6.2 KB) - added by novoselt 7 years ago.
trac_14382_doctests.patch (6.2 KB) - added by novoselt 7 years ago.
trac_14382_fix_latex_of_strings.patch (4.4 KB) - added by novoselt 7 years ago.
Use text-tt
trac_14382-texttt.patch (10.4 KB) - added by jhpalmieri 7 years ago.
use \text{\texttt{...}}
trac_14382-delta.patch (6.2 KB) - added by jhpalmieri 7 years ago.
for review only: diff between previous patches and texttt patch

Download all attachments as: .zip

Change History (41)

comment:1 Changed 7 years ago by ddrake

  • Cc ddrake added

comment:2 Changed 7 years ago by jhpalmieri

  • Cc jhpalmieri added

comment:3 Changed 7 years ago by novoselt

  • Authors set to Andrey Novoseltsev
  • Cc cremona added
  • Keywords latex added
  • Status changed from new to needs_review

comment:4 Changed 7 years ago by nthiery

You might want to have a look at #13624; it replaces \verb back to \text to help dot2tex. Not really your business, but maybe it would make sense to use \text in the easy cases, with no \phantom's? This is quite more readable!

Cheers,

Nicolas

comment:5 Changed 7 years ago by novoselt

I don't think \phantom{x} is likely to cause troubles, and I didn't figure out a better way to insert a "monotype width space" compatible with MathJax we use. The method in question here is fallback to when classes do not produce latex code at all, and in this case using verb is the best we can do to preserve formatting: keep line breaks and vertical alignment in case it is important. In particular cases like #13624 there has to be proper latex treatment of strings. The general case has to be uniform, I think, not like in #14256.

comment:6 Changed 7 years ago by jhpalmieri

Can you provide examples that this is supposed to fix? Running SageTeX on a file containing

\[
\sage{RubiksCube()}
\]

doesn't work either before or after the patch.

comment:7 Changed 7 years ago by jhpalmieri

By the way, line 337 should say MathJax, not jsMath.

comment:8 Changed 7 years ago by jhpalmieri

Here's a LaTeX file:

\documentclass{article}
\usepackage{sagetex}
\begin{document}

Let's try this:
\[
\sage{"1 2"}
\]
Doesn't work.

\end{document}

Running latex on this, then sagetex, then latex, I get this error:

! Missing } inserted.
<inserted text> 
                }
l.7 \sage{"1 2"}
                
? 

comment:9 Changed 7 years ago by ddrake

As long as you have verbatim things, this won't work in TeX documents. The \sage macro relies on pulling in the output via a label, and verbatim things don't work well when pushed around with labels and references. (They are "fragile".)

I mentioned this on another ticket, but one problem we have is that we use the LaTeX output for two different environments: for consumption by MathJax?, and by regular LaTeX. Those environments are really pretty different.

comment:10 Changed 7 years ago by novoselt

OK, my bad, it still does not work with SageTeX, althoug the generated code is at least valid in LaTeX for copy-pasting, while putting verb inside of phantom is not.

After playing some more, I see that MathJax that we have does support \tt, so I'll try to use that instead of verb.

comment:11 follow-up: Changed 7 years ago by jhpalmieri

I don't think that \tt will handle spaces correctly, but maybe I'm wrong.

One option we could consider: use EMBEDDED_MODE to detect whether we're using the notebook. Or we could produce output that SageTeX can handle but not MathJax, and then post-process it if using MathJax, converting the relevant string, maybe in the eval method for MathJax. We used to do this with jsMath, if I recall correctly.

comment:12 Changed 7 years ago by novoselt

Actually one of the reasons for using verb was that strings may contain all kinds of special characters and it is tricky to display them all properly. So I think that for MathJax we still are better off using verb, but perhaps in non-embedded mode something else can be done, although special characters still have to be allowed.

A combination of tt and phantom in array can produce proper spacing, it seems, but somehow special characters have different width when escaped, e.g. #

comment:13 Changed 7 years ago by novoselt

  • Status changed from needs_review to needs_work
  • Work issues set to try text/tt

Ahhh, it seems that \text{\tt ...} works quite well, I guess with \tt in math mode escaping was switching to a different font. Will try to make another patch this week.

comment:14 in reply to: ↑ 11 Changed 7 years ago by nthiery

Replying to jhpalmieri:

One option we could consider: use EMBEDDED_MODE to detect whether we're using the notebook.

I did not really think about it, but even in EMBEDDED_MODE one may want to produce latex code for something else than mathjax. So maybe that's not super robust.

comment:15 Changed 7 years ago by novoselt

OK, how about this one: old-style verb in EMBEDDED_MODE and text-tt wrapping otherwise. RubiksCube() looks perfect when rendered through SageTeX! (I didn't change documentation/doctests yet.)

Note that both ways generate valid LaTeX code, although verb one does not work in command arguments and text-tt does not work in MathJax... Nevertheless I think it is the way to go - particular cases where this treatment is insufficient should provide adequate handling of LaTeX generation themselves.

comment:16 Changed 7 years ago by jhpalmieri

I think Nicolas's point has some merit. Could we instead have text-tt wrapping in general, and then in the MathJax eval method, do a regular expression search-and-replace to change \text{\tt{...}} into \verb|...|, or something like that?

comment:17 Changed 7 years ago by novoselt

  • Status changed from needs_work to needs_review
  • Work issues try text/tt deleted

New version uses text-tt and switches to verb in MathJax. I don't think that regular expressions can be used, since it is necessary to balance braces. There was a code in free modules removing the old verb wrappers - I switched it to using string representation in this case. If this is not what is desired, then I think no fiddling has to be done here at all, since without verb there are no issues with command arguments.

comment:18 Changed 7 years ago by novoselt

Also, since verb is now used for MathJax only, I left \phantom{\verb which I complained about originally - it is not a valid LaTeX still, but it does give better look in MathJax - RubiksCube() looks now perfectly both in the notebook and via SageTeX.

Changed 7 years ago by novoselt

Changed 7 years ago by novoselt

comment:19 Changed 7 years ago by novoselt

  • Description modified (diff)

Fixed the doctest in tutorials which I missed before (by the way, the patchbot caught the problem in the English version only, not German).

comment:20 Changed 7 years ago by novoselt

Apply trac_14382_fix_latex_of_strings.patch trac_14382_doctests.patch

comment:21 Changed 7 years ago by novoselt

comment:22 Changed 7 years ago by novoselt

Spaces in verb are handled correctly with 2.2 in SVG mode, but are still quite off in HTML-CSS, so I think that patches here are still the way to go!

comment:23 Changed 7 years ago by jhpalmieri

With this patch, running view(RubiksCube()) from the command line produces output in which the numbers are lined up, but the horizontal lines are too short: LaTeX is turning each pair of dashes -- into an "en-dash". If you want to use this approach, maybe you need to use {-}{-} instead of --.

From the notebook, I don't see anything useful, but that may be specific to this machine. I'll try it on another machine eventually.

From SageTeX, it works, but it requires the package amsmath because of the \text{...} command. I'm not sure that's ideal.

comment:24 Changed 7 years ago by novoselt

  • Status changed from needs_review to needs_work
  • Work issues set to dashes and commanline view

Interesting, was working fine for me except for the command-line which I didn't think about. Will keep working.

comment:25 follow-up: Changed 7 years ago by novoselt

Does anyone recall any reason to use \text{\tt } rather than \texttt{ }? The latter should be eventually supported by MathJax (i.e. they plan to do it) while the former is a more general problem of not supporting nested font switches or something like that. Directly from LaTeX it seems to be identical.

I am also puzzled by combining of dashes - it does not happen for hand-written code or via !SageTeX, why does it happen for view???

comment:26 in reply to: ↑ 25 Changed 7 years ago by jhpalmieri

Probably use \mathtt instead of \texttt, since LaTeX output in Sage should be in math mode. Otherwise, I don't know why use \text{\tt } instead of \mathtt. I don't understand the dash issue, either.

Last edited 7 years ago by jhpalmieri (previous) (diff)

comment:27 Changed 7 years ago by novoselt

\mathtt is not a monospaced font. I can't see the difference between \text{\tt } and \texttt{ }, but the latter works without amsmath. On the other hand, without amsmath it does not scale properly, so amsmath is still desirable. I don't really see the problem with it - hard to imagine that someone will use Sage for typesetting math and yet object to loading a standard math package.

comment:28 Changed 7 years ago by jhpalmieri

You shouldn't use \texttt{...} within math mode. It might work, but if so, it's taking advantage of a bug.

comment:29 Changed 7 years ago by novoselt

OK, then it seems to me that missing dashes for the command line are the only issue!

Changed 7 years ago by novoselt

Use text-tt

comment:30 Changed 7 years ago by novoselt

  • Status changed from needs_work to needs_review
  • Work issues dashes and commanline view deleted

Didn't run tests yet, but they should not be affected by this change, hopefully.

comment:31 Changed 7 years ago by novoselt

All tests pass both for me and the patchbot.

John, what exactly was "From the notebook, I don't see anything useful"? Was it indeed something with a single machine or some generic issue? I've had these patches on for a while and didn't experience any problems under Windows and Linux.

Changed 7 years ago by jhpalmieri

use \text{\texttt{...}}

Changed 7 years ago by jhpalmieri

for review only: diff between previous patches and texttt patch

comment:32 Changed 7 years ago by jhpalmieri

  • Description modified (diff)

Looks okay except that \tt is an obsolete command, and \texttt should be used instead. I've attached a new patch which does this, along with a delta patch which shows the difference between my patch and your two patches. All tests pass for me and the output looks the same as with \tt.

comment:33 Changed 7 years ago by novoselt

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

Wow, I thought LaTeX is once and for all and only new things can appear without obsolescence! Will keep it in mind for the future, changes are fine with me!

comment:34 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:35 Changed 7 years ago by novoselt

Apply trac_14382-texttt.patch​

comment:36 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.12.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.