metamath / metamath-exe

Metamath program - source code for the Metamath executable
GNU General Public License v2.0
77 stars 25 forks source link

add handling of double underscores in generated html #160

Closed benjub closed 1 year ago

benjub commented 1 year ago

Almost certainly buggy since I don't know the codebase nor the language. Also, probably inefficient since I copied code from a more complex case than just replacing __ with _. This is more like pseudocode than anything else, actually, but I wanted to get the ball rolling since the need for this feature was emphasized in https://github.com/metamath/set.mm/pull/3389#discussion_r1298845431 by @avekens (i.e., we want to display command names like MINIMIZE_WITH and TRACE_BACK without triggering italics or subscripts in the generated html).

Help would be appreciated (it may be simpler to start another PR). Maybe from @digama0 or @wlammen or @tirix ?

tirix commented 1 year ago

I've added some comments, but that's a lot of different changes. I could not propose changes directly in the repository (maybe I don't have the rights for that). What I came up with is:

      /* Double-underscore handling: double-underscores are "escaped
      underscores", so replace a double-underscore with a single
      underscore and do not modify italic or subscript. */
      if (cmt[pos1] == '_') {
            if (g_htmlFlag) {  /* HTML */
              let(&cmt, cat(left(cmt, pos1), /* Skip (delete) "_" */
                  right(cmt, pos1 + 2), NULL));
              let(&cmtMasked, cat(left(cmtMasked, pos1), /* Skip (delete) "_" */
                  right(cmtMasked, pos1 + 2), NULL));
              pos1 ++; /* Adjust for 1 extra char '_' */
            } else {  /* LaTeX */
              let(&cmt, cat(left(cmt, pos1 - 1),  /* Skip (delete) "_" */
                  "\\texttt{\\_}",
                  right(cmt, pos1 + 2), NULL));
              let(&cmtMasked, cat(left(cmtMasked, pos1 - 1),  /* Skip (delete) "_" */
                  "\\texttt{\\_}",
                  right(cmtMasked, pos1 + 2), NULL));
              pos1 = pos1 + 11; /* Adjust for 11 extra chars "\texttt{\_}" */
            }
        continue;
      } /* End of double-underscore handling */

I've tested it successfully with HTML mode, but it does not work for TEX as the whole thing seems to be skipped at line 1941 with a test on the g_htmlFlag global variable.

digama0 commented 1 year ago

needs a test

benjub commented 1 year ago

Added Thierry's fix and tests.

benjub commented 1 year ago

As for the tex: the if (g_htmlFlag) at Line 1970 of mmwtex.c is inside the if (g_htmlFlag != 0 && at Line 1941, and there semms to be no side effects affecting g_htmlFlag in between, so the else branch beginning Line 1976 is probably useless. (Same thing for Lines 2013/2024, and 2049/2057, which are not part of this PR.)

tirix commented 1 year ago

As for the tex: the if (g_htmlFlag) at Line 1970 of mmwtex.c is inside the if (g_htmlFlag != 0 && at Line 1941, and there semms to be no side effects affecting g_htmlFlag in between, so the else branch beginning Line 1976 is probably useless. (Same thing for Lines 2013/2024, and 2049/2057, which are not part of this PR.)

That one is probably best left aside for now. This is not directly related with this PR. One would have to understand if annotations are handled somewhere else for LaTeX (and those cases don't need to be handled here) or not (and special care has to be taken for LaTeX in that case too).

benjub commented 1 year ago

Reverted the last commit, as @tirix suggested. This looks ready to me. Better reviewed as a whole, since there was a commit revert. @digama0 ?

benjub commented 1 year ago

@digama0 does it look good to you ?

By the way: if we keep tests/.gitignore, we should add *.produced to it and remove it from .gitignore, or else we only keep a single ./gitignore at the root and put everything there.

benjub commented 1 year ago

@digama0 does it look good to you ?

digama0 commented 1 year ago

LGTM, I didn't check the replacement logic in detail but I'm happy to follow up with bugfixes if we find any issues later. This should be replicated in metamath-knife if it hasn't been done already.

benjub commented 1 year ago

This should be replicated in metamath-knife if it hasn't been done already.

Not to my knowledge... and I'm not that fluent in Rust. Maybe @tirix knows/can add it ?

tirix commented 1 year ago

Maybe @tirix knows/can add it ?

The first part is in metamath/metamath-knife#127.