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

Allow loading LaTeX user-defined macros from text files #27

Merged
merged 2 commits into from
Feb 15, 2016

Conversation

davethecipo
Copy link
Contributor

Hi,
thanks for the plugin first of all, it's very useful!

You may know that Mathjax supports custom macros, by writing them in LaTeX syntax inside $ $ or $$ $$, but also by adding an option Macros to MathJax.Hub.Config, in javascript style of course.

I usually find myself using the same definitions, in order to avoid repetitions I added a function that parses and loads definitions from tex files.

I'm not a professional programmer, therefore suggestions to improve the code are welcome.

@barrysteyn
Copy link
Owner

Hi, I am very sorry I have taken so long to get back to you. Thank you, this seems amazing and very cool. The one thing I wish to add is that I have no tests on this project. Originally, I wrote it for me, and it just grew! So I guess what I want to ask you is, can you tell me what you have done to test your feature?

@davethecipo
Copy link
Contributor Author

Hi!

I created two files with definitions (some of them were repeated). I created two virtual environments, seems to work for both python 2.7 and 3.4.

I'd love to put some unit tests around the code in the next commit, just to be sure before merging.

Do you have some preferences for the unit test suite? Standard unittest, nose, pytest? In any case I would use tox to ensure compatibility for both 2.7 and 3.4 (and other versions too if needed).

As last remark, I'd point out that the function just reads the input file without checking for bad input, spitting out garbage for malformed input. I'm not sure how to handle this, I thought about the following possibilities

  1. inform the user in the README about the fragility of the parsing, without adding sanity checks. Therefore one should check the website locally before uploading.
  2. sanity check for example by using pyparsing

math.py is renamed to avoid collisions with the standard library module.

The function handling macros has been refactored for better testability
and maintainability.
@davethecipo
Copy link
Contributor Author

Hello again,

I noticed that the warning gets printed multiple times, which is confusing for the user. I assume that this behavior is due to the signaling mechanism of Pelican, do you have any hints for me?

@barrysteyn
Copy link
Owner

Hi @davethecipo

Just to say that I am allotting some time this weekend to dive into this. I am sorry it is taking so long, but you have caught me at a particularly busy time.

@davethecipo
Copy link
Contributor Author

Hello again,

I'm glad you got some time to take a look at this.

In the meanwhile, I found out why I get the warning message printed multiple times: I enabled the i18n_subsites plugin. I should probably ask its developer for info.

In my code I just used the old simple print, it's probably better to use the logging module

@barrysteyn
Copy link
Owner

Hi

I have looked at your changes and I am going to accept the pull request. I will merge it here, and then submit it to the main repo.

@barrysteyn barrysteyn closed this Feb 15, 2016
@barrysteyn barrysteyn reopened this Feb 15, 2016
barrysteyn added a commit that referenced this pull request Feb 15, 2016
Allow loading LaTeX user-defined macros from text files
@barrysteyn barrysteyn merged commit 2bb931a into barrysteyn:master Feb 15, 2016
@danmackinlay
Copy link

Huh! I wish I saw this one sooner and I wouldn't have implemented macro support in a different way in my repo

Looking over at the two approaches, I would say that the relative advantage the @davethecipo made

  • test coverage
  • attempts to validate user input
  • allows input of macros in vanilla TeX instead of JS

the relative advantages of the patch i made

  • simpler implementation of output
  • backslashes do not need multiple-escaping
  • does not require hand-rolled TeX parser

I of course cannot weigh up these advantages for everyone else.

I should mention, though, a third option --- you could get the simplicity of my patch and the TeX-native feel of @davethecipo 's patch by loading a .tex-file into a hidden math-block at the start of the page.
This leverages the MathJaX parser and requires less code.

@danmackinlay
Copy link

Geeze, I didn't even thank @barrysteyn and @davethecipo for their work here. So rude! Thanks, folks.

@barrysteyn
Copy link
Owner

I love simplicity. The third option that you are mentioning seems very attractive to me... Can you flesh it out some more.

@danmackinlay
Copy link

There is an example of both methods on the MathJax site
The alternative method is to preface the document with an invisible math div that declares your macros.
Since we are already inserting script nodes after the document body, this is not substantially more work than render_math already does.

Example preface:

<div style="display:none">
\(
  \def\<#1>{\left<#1\right>}
  \newcommand{\CC}{\mathbf{C}}
\)
</div>
<article>
    Blah blah blah...

I am likely to have time to look at this over easter.

@davethecipo
Copy link
Contributor Author

@howthebodyworks your third option looks great to me too, I overlooked that section of the MathJax documentation.

@danmackinlay
Copy link

Happy to prepare a patch to implement that; NB it would be a different (and simpler) patch if getpelican/pelican-plugins#693 goes through, so I'm waiting on that.

@danmackinlay
Copy link

A workaround which I currently use for this is simply to define macros on the page itself inside $$...$$, which works fine

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.

3 participants