Attempting to fix the curly brackets bug #166
Conversation
src/mmwtex.c
Outdated
| // k counts the scope level we are in. | ||
| k = 0; | ||
| n = (long)strlen(texFull); | ||
| for (j = 1; j < n; j++) { // We don't need to check texFull[0]. |
There was a problem hiding this comment.
Here I wasn't sure what apporach would be best. I defined the texFull string as the line that is feeded to the printLongLine function. The checking cicle starts at the second character of texFull to avoid checking the unorthodox texFull[-1] != '\\'. The first character is texFull[0] = " " so we don't need to check it since it's fixed.
| htmRef, NULL), | ||
| " \\notag \\\\ && & \\qquad ", // Continuation line prefix | ||
| " "); | ||
| texFull = cat(" ", // texFull[0] should not be a "{" character. |
There was a problem hiding this comment.
Here I added a comment to point out that texFull[0] = " " should not be a "{" character, since it wouldn't be checked by the subsequent counting algorithm.
There was a problem hiding this comment.
Ok, and here it obviously will never be "{", since it is a blank space " ".
src/mmwtex.c
Outdated
| // To avoid generating incorrect TeX, line breaking is forbidden inside | ||
| // scopes of curly braces. However breaking between '\{' and '\}' is | ||
| // allowed. | ||
| // The spaces that should not be matched with 'breakMatch' are | ||
| // temporarily changed to ASCII 3 before the 'printLongLine' procedure | ||
| // is called. The procedure rewraps the 'line' argument matching the | ||
| // unchanged spaces only, thus ensuring bad breaks will be avoided. | ||
| // The reverse is done in the 'print2()' function, where all ASCII 3's | ||
| // are converted back to spaces. | ||
| // k counts the scope level we are in. | ||
| k = 0; | ||
| n = (long)strlen(texFull); | ||
| for (j = 1; j < n; j++) { // We don't need to check texFull[0]. | ||
| // We enter a non "\{" scope. | ||
| if (texFull[j] == '{' && texFull[j - 1] != '\\') k++; | ||
| // We escape a non "\}" scope. | ||
| if (texFull[j] == '}' && texFull[j - 1] != '\\') k--; | ||
| // If k > 0 then we are inside a scope. | ||
| if (texFull[j] == ' ' && k > 0) texFull[j] = QUOTED_SPACE; | ||
| } | ||
| printLongLine(texFull, " \\notag \\\\ && & \\qquad ", /* Continuation line prefix */ " "); |
There was a problem hiding this comment.
Looks like a lot of duplication with the previous part.
Then again the duplicated code was already there before.
There was a problem hiding this comment.
I unduplicated the brace count and the printLongLine call in a new commit.
|
Maybe one (last?) remark would be to integrate your minimal example into an additional test in the Unfortunately I don't have the rights to merge, I guess @digama0 does. |
|
My only question is whether |
|
Oh right, of course I have not thought so far. |
|
There is another test which has to check the result of an auxiliary file, and uses a trick calling |
|
I don't understand what's wrong. On my computer both attempts pass. Maybe it doesn't like backslashes? |
|
Based on this SO answer, line 113 of |
|
(Accidentally pressed the close button) Thanks for making it work, should be good to merge now. I wonder if there is any difference between using |
|
Btw sorry @tirix I superficially read your suggestion and made a mess. |
|
The final one looks good! |
This is my attempt to fix issue #129. The solution I proposed in #129 (comment) is actually not very practical because latex translations get merged togheter pretty early and this makes it difficult to take advantage of that information. So I changed my mind and implemented @david-a-wheeler proposal instead #129 (comment). I took inspiration from an analogous functionality already present for HTML:
metamath-exe/src/mminou.c
Lines 549 to 588 in ae35c1b
With this change the generated TeX of my minimal example seems to work as intended:
% This LaTeX file was created by Metamath on 22-Oct-2023 9:40 PM. \documentclass{article} \usepackage{amssymb} % amssymb must be loaded before phonetic \usepackage{phonetic} % for \riota \usepackage{mathrsfs} % for \mathscr \usepackage{mathtools} % loads package amsmath \usepackage{amsthm} % amsthm must be loaded after amsmath \usepackage{accents} % accents should be loaded after mathtools \theoremstyle{plain} \newtheorem{theorem}{Theorem}[section] \newtheorem{definition}[theorem]{Definition} \newtheorem{lemma}[theorem]{Lemma} \newtheorem{axiom}{Axiom} \allowdisplaybreaks[1] % Allow page breaks in {align} \usepackage[plainpages=false,pdfpagelabels]{hyperref} \hypersetup{colorlinks} % Get rid of boxes around links \begin{document} \begin{proof} \begin{align} 1 && & \text{ A A A A A A A A A A } \text{ A A A A A A A A A A } \notag \\ && & \qquad \text{ A A A A A A A A A A } \notag \\ && & \qquad \text{ A A A A A A A A A A } \text{ A A A A A A A A A A }&(\mbox{\ref{eq:ax-1}})\notag \end{align} \end{proof} \end{document}I'm somewhat surprised by how simple this solution looks, and this makes me wonder whether it might be a false friend due to my own ignorance. So it most likely needs to be overhauled by an expert eye.