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

math2svg.lua filter #118

Merged
merged 51 commits into from
Jan 16, 2021
Merged

math2svg.lua filter #118

merged 51 commits into from
Jan 16, 2021

Conversation

stroobandt
Copy link
Contributor

@stroobandt stroobandt commented Oct 11, 2020

This Lua filter converts LaTeX math to MathJax generated SVG in any of the available MathJax fonts.

Serge Y. Stroobandt added 10 commits October 11, 2020 17:56
	new file:   displaymath2svg.lua
        modified:   displaymath2svg.lua
        modified:   displaymath2svg.lua
        new file:   .gitignore
        modified:   README.md
        modified:   displaymath2svg.lua
        modified:   displaymath2svg.lua
        new file:   Makefile
        new file:   expected.html
        new file:   sample.md
        modified:   README.md
        modified:   displaymath2svg.lua
@tarleb
Copy link
Member

tarleb commented Oct 11, 2020 via email

@stroobandt stroobandt closed this Oct 11, 2020
@stroobandt stroobandt reopened this Oct 11, 2020
@stroobandt
Copy link
Contributor Author

stroobandt commented Oct 11, 2020

Thank you for the very constructive comments.
I will work on these.

Serge Y. Stroobandt added 2 commits October 12, 2020 01:01
        modified:   README.md
        modified:   displaymath2svg.lua
@stroobandt stroobandt changed the title displaymath2svg.lua filter math2svg.lua filter Oct 12, 2020
@stroobandt
Copy link
Contributor Author

Dear Albert,

I think I have been able to attend to all your concerns:

  • The path to tex2svg has been isolated in a configuration variable. A comment explains how to locate this executable.
  • The filter has been licensed under the MIT license.
  • The newcommands variable is now empty by default. An example is provided in the comments.
  • When the output format is not HTML, the SVG is now inserted into the mediabag.
  • The program can now also convert inline math to SVG. The type of math that gets converted, display versus inline, is determined by two new configuration variables. Inline math conversion to SVG is switched off by default and falls back to MathML. The reasons for this are explained in the comments.
  • The name of the filter changed to math2svg.lua to reflect the changes in the point above.

Copy link
Member

@tarleb tarleb left a comment

Choose a reason for hiding this comment

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

Thank you for the quick update. There are a few things which I forgot to include in my first comment, and was not specific enough about others. See below for a couple of inline comments.

Cheers!

math2svg/.gitignore Outdated Show resolved Hide resolved
math2svg/README.md Outdated Show resolved Hide resolved
math2svg/README.md Show resolved Hide resolved
math2svg/README.md Outdated Show resolved Hide resolved
math2svg/README.md Outdated Show resolved Hide resolved
math2svg/math2svg.lua Outdated Show resolved Hide resolved
math2svg/math2svg.lua Outdated Show resolved Hide resolved
math2svg/math2svg.lua Outdated Show resolved Hide resolved
math2svg/math2svg.lua Outdated Show resolved Hide resolved
math2svg/math2svg.lua Outdated Show resolved Hide resolved
@stroobandt
Copy link
Contributor Author

stroobandt commented Oct 17, 2020

You could try to do the latter yourself by editing .travis.yml if you'd like

Done. It seems to be working. I also cleaned up a bit the YAML indenting as per Travis CI documentation.

  • Couldn't we set the default for math2svg_path to just tex2svg? This would allow users to simply add the executable into their PATH and would make the filter truly platform independent. What do you think about naming that parameter tex2svg_path to make it more obvious what it's used for?

Done. The key is now called math2svg_tex2svg. When absent, the filter will now just try to execute tex2svg; rendering it truly platform independent.

  • Some lines are very long, both in the filter and the readme. Lines should stay below 80 chars if possible.

Reducing the lines below 80 columns is simply not possible because some of the hyperlinks in README.md are longer than that.
However, I was able to reduce README.md and math2svg.lua to below 120 columns, which also happens to be a terminal standard. (Hey, we are not 1984 anymore! — Although it certainly feels like it nowadays.)

To set the record straight: More than a year before John was even considering this, others and myself were already playing with math SVG. Credits for suggesting the current solution go out to Nikolay "Lierdakil" Yakimov.

Anyhow, no, I do not think this needs mentioning. What I do think is that our Lua filters deserve BETTER MARKETING.
I will open a separate issue about this for discussion. Currently. Lua filters look more like an afterthought at the bottom of the Extras page, when they are actually easier to maintain than Haskell filters. When one eventually lands on this README.md, it becomes a GitHub dumpster dive to find out which Lua filter does what. I would suggest adding one paragraph descriptions to the general README.md and do something similar on the Extras page.

  • If we were to set math2svg_inline2svg to true by default, then the filter could serve as a full drop-in replacement for pandoc-tex2svg. Personally, I'd also find this more consistent, but it is your choice.

Won't fix. In the README.md I promise the user sensible defaults. Even John complains about the filter being slow when both display and inline math are being converted:

"The filter is rather slow, and it adds significantly to file size, but the resulting HTML renders quickly and does not depend on an internet connection or JavaScript."

  • I remembered that HTML does not allow <div> elements within <p> elements. The filter currently produces invalid markup and should probably use <span> for both "display" and "inline" elements. We could think about adding style="display:block" to "display" spans, but maybe it's better to leave that to a global CSS rule.

Work in progress. I already fixed it with a <span class="math display">. However, since I eat my own dog food, I discovered this is not ideal for CSS paged media processing. I will run some tests to see what works best, whilst keeping the HTML valid. I will also document this in the README.md.

@stroobandt
Copy link
Contributor Author

I opened #121 concerning better Lua filter marketing.

@tarleb
Copy link
Member

tarleb commented Oct 24, 2020

The additional, non-essential changes to .travis.yml broke the config. PRs should only have one topic. Changing indentation in a config file is a separate concern and should be treated accordingly.

The 80 char-per-line rule is rooted in two principles: (1) The README should be easy to read, no matter whether viewed as rendered HTML in a browser, or as plaintext on a console. (2) research showed that text is easiest to read if there a 60 to 66 characters per line. 80 is still acceptable, anything above 100 is seriously bad. If you don't like how links look when forcing shorter lines, use reference links.

math2svg/README.md Outdated Show resolved Hide resolved
@tarleb
Copy link
Member

tarleb commented Nov 22, 2020

We had to switch from Travis CI, so now all tests are run in GitHub Actions. This led to a merge conflict which has to be resolved before we can finally merge. The additional packages now need to be added in .tools/Dockerfile. Could you update the PR?

@stroobandt
Copy link
Contributor Author

@tarleb Dear Albert, I have modified .tools/Dockerfile, but apparently I cannot test it because of lacking write permissions.

Only those with write access to this repository can merge pull requests.

The packages nodejs and npm are required for testing.
There is also an additional RUN statement to download an npm package.

@stroobandt
Copy link
Contributor Author

All open issues have been resolved.
If an administrator can take care of the suggested .tools/Dockerfile,
this new Lua filter should be ready to go.

@tarleb tarleb merged commit c705b8f into pandoc:master Jan 16, 2021
@tarleb
Copy link
Member

tarleb commented Jan 16, 2021

Thanks!

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