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

Make \require{bussproofs} work properly. #408

Merged
merged 1 commit into from
Jan 24, 2020
Merged

Make \require{bussproofs} work properly. #408

merged 1 commit into from
Jan 24, 2020

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Dec 17, 2019

Do a restart if a package loaded with \require has preprocessors (since they will need to run before the math is processed). This allows \require{bussproofs} to be in the same expression as the proof itself. (The Bussproofs package is the only one that uses preprocessors at the moment.)

Also, remove dupolication of pre- and post-processors from the register() function, since these are already added during the config.config() call just before that.

Without this patch,

$$
\require{bussproofs}
\begin{prooftree}
  \AxiomC{C}
  \AxiomC{A}
  \AxiomC{B}
  \BinaryInfC{D}
  \BinaryInfC{E}
\end{prooftree}
$$

would produce an error if the bussproofs extension was not loaded and included in the package list initially.

…ce they will need to run before the math is processed). This allows \require{bussproofs} to be in the same expression as the bussproof itself. Also, remove dupolication of pre- and post-processors from the register() function.
@dpvc dpvc requested a review from zorkow December 17, 2019 21:09
Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

That doesn't work for me. I have not investigated in detail, why; but without these changes your example with require works fine.

@zorkow
Copy link
Member

zorkow commented Jan 24, 2020

Upon further investigation: It does work but only after an explicit typeset in the lab. I.e., not on reload.

@dpvc
Copy link
Member Author

dpvc commented Jan 24, 2020

This program:

MathJax = {
  loader: {
    load: ['adaptors/liteDOM'],
    paths: {mathjax: './es5'},
    require: require
  },
  startup: {
    ready() {
      MathJax.startup.defaultReady();
      return MathJax.tex2mmlPromise(`
\\require{bussproofs}
\\begin{prooftree}
  \\AxiomC{C}
  \\AxiomC{A}
  \\AxiomC{B}
  \\BinaryInfC{D}
  \\BinaryInfC{E}
\\end{prooftree}`)
        .then((x) => console.log(x));
    }
  }
};

require('./es5/tex-chtml.js');

when run in the MathJax-src root directory (making sure to build the es5 directory first) fails to run without this PR and does run with it for me. Can you check if this works for you?

@dpvc
Copy link
Member Author

dpvc commented Jan 24, 2020

[Looks like our messages crossed.]

Upon further investigation: It does work but only after an explicit typeset in the lab. I.e., not on reload.

It works for me with reload of the lab, with typeset, and with "keep". What message are you getting in the console?

@dpvc
Copy link
Member Author

dpvc commented Jan 24, 2020

Without this PR, I get:

TypeError: "doc is null"
    getBBox BussproofsUtil.ts:56
    adjustValue BussproofsUtil.ts:244
    balanceRules BussproofsUtil.ts:509
    execute FunctionList.ts:51
    executeFilters InputJax.ts:197
    compile tex.ts:200
    compile MathItem.ts:327
    compileMath MathDocument.ts:680
    compile MathDocument.ts:655
    methodActions MathDocument.ts:166
    renderDoc MathDocument.ts:180
    render MathDocument.ts:606
    typesetPromise startup.ts:335
    run Retries.ts:73
    handleRetriesFor Retries.ts:71
    typesetPromise startup.ts:334
    Typeset v3-lab.js:201
    newPackages v3-lab.js:307
    Startup v3-lab.js:623
    ready v3-lab.js:115
    defaultReady loader.ts:140
    execute startup.js:14

With the PR, everything works as expected for me.

@dpvc
Copy link
Member Author

dpvc commented Jan 24, 2020

Oh, I forgot that I had adding some error trapping in my copy of the lab. I added

options: {
    compileError(doc, math, err) {console.log(err); throw err},
    typesetError(doc, math, err) {console.log(err); throw err}
},

to the MathJax configuration in order to get better error message reports.

@zorkow
Copy link
Member

zorkow commented Jan 24, 2020

I've tested it in a standalone file now. And I can confirm that it works with the PR (not without).
Maybe my lab is just messed up.

Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

Works fine now.

@dpvc
Copy link
Member Author

dpvc commented Jan 24, 2020

Maybe my lab is just messed up.

Which branch (of the MathJax-dev repository) are you working on? I'm on the update-tests branch, not the master branch.

@zorkow
Copy link
Member

zorkow commented Jan 24, 2020

Oh, I forgot that I had adding some error trapping in my copy of the lab. I added

options: {
    compileError(doc, math, err) {console.log(err); throw err},
    typesetError(doc, math, err) {console.log(err); throw err}
},

to the MathJax configuration in order to get better error message reports.

Are you still using the one in the dev repo, in the v3-tools branch?

@dpvc
Copy link
Member Author

dpvc commented Jan 24, 2020

OOPS, I see you have it working now.

@dpvc
Copy link
Member Author

dpvc commented Jan 24, 2020

Are you still using the one in the dev repo, in the v3-tools branch?

Yes and no. I have a private copy in the MathJax-src directory that I mostly use, but I did test the one in the dev repository to see if that was it. Both worked as expected for me.

@dpvc dpvc merged commit dd882db into develop Jan 24, 2020
@dpvc dpvc deleted the better-require branch January 24, 2020 22:16
@dpvc dpvc added this to the 3.0.1 milestone Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants