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

Implement LaTeX math support (WIP; Please do not merge this pull request yet) #42

Closed
wants to merge 13 commits into from

Conversation

dakusui
Copy link

@dakusui dakusui commented Oct 28, 2018

This is a pull request that offers an implementation of Issue-#39 (LaTeX math mode support)
NOTE: Please do not look into this pull request anymore. Refer to this one for review.
NOTE: This pull request is opened for review and discussions. Another pull request will be created when it becomes necessary.

README

Following is a documentation about this feature. This will appear in README.md after discussions are concluded.

Usage and syntax

Command line

(snip)

 -e,--encoding <ENCODING>       The encoding of the input file.
 -h,--html                      In this case the input is an HTML file.
                                The contents of the <pre
                                class="textdiagram"> tags are rendered as
                                diagrams and saved in the images directory
                                and a new HTML file is produced with the
                                appropriate <img> tags.
    --help                      Prints usage help.
-L,--latex-math                 Enable LaTeX math mode.

(snip)

Syntax

(snip)

LaTeX mode.

If you place LaTeX formulae inside 2 $s, it will be rendered using jlatexmath. That is, if you have a following input files.


$Box_1$                    $Box^2$
+---------------------+    +------+   /---------\
|$\sum_{i=0}^{n}x^i$  |    |$cBLU$|   |         |
|                     +--->|cRED  +-=-+cGRE$C_k$|
|{io}                 |    |cXYZ  |   |{o}      |
+----------+----------+    +---+--+   \---------/
           |                   |
           |                   :
           |                   V
           |           +-------------------+
           +---------->*$A_i$ hello $B^i$  |
                       |              +----+
                       |              |c8FA|
                       +--------------+----+

$|Set| = o-*-Freunde-*-nicht=*=diese-=-*- * töne$

o Quick brown fox jumps over
* a lazy dog.

$Q_u^i$, $C_k$, $B_r^{own}$, $F_{ox}$ jumps

over a lazy $d\cdot\frac{o}{g}$.


$\forall x \in X, \quad \exists y \leq \epsilon$


$\sin A \cos B =$

    $ \frac{1}{2}\left[ \sin(A-B)+\sin(A+B) \right]$


$\frac{d}{dx}\left( \int_{0}^{x} f(u)\,du\right)=f(x).$


 $v \sim \mathcal{N} (m,\sigma^2)$

This will be rendered as follows.

art-latexmath-1

Limitations

This feature is only available when you are generating .png files.

Discussions

  • This enhancement introduces a new semantics to a certain character in a text and it means potentially hurts a compatibility. I think I can implement a new option in command line to enable/disable this feature if you think it is important. Please let me know your thought.
  • I think .svg support should be done in a separate pull-request since it would be a bit big change.

Tasks

  • Implement tests
  • Fix a bug where * is rendered as a bullet point (or a connector?) even if it is placed inside $ and $.
  • Remove pom.xml
    • Backport chages made in pom.xml to project.clj
  • Provide a documentation.
  • Implement an option to enable this feature. This must be disabled by default.

@dakusui dakusui changed the title Latex math support (WIP; Please do not merge yet) Implement Latex math support (Issue-#39; WIP; Please do not merge this pull request yet) Oct 28, 2018
@dakusui dakusui changed the title Implement Latex math support (Issue-#39; WIP; Please do not merge this pull request yet) Implement Latex math support (WIP; Please do not merge this pull request yet) Oct 28, 2018
import org.stathissideris.ascii2image.text.TextGrid;

import javax.imageio.ImageIO;
import java.awt.*;
Copy link
Owner

Choose a reason for hiding this comment

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

Please keep the explicit imports from before, it will help with the transition to Clojure.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, apparently my IDE's setting is too much aggressive at formatting imports, let me fix it.


public CellSet getAllNonBlank() {
CellSet set = new CellSet();
int w
Copy link
Owner

@stathissideris stathissideris Oct 28, 2018

Choose a reason for hiding this comment

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

Fixing whitespace is great, but please put such changes in a different pull request as they obscure the substance of your contribution and make the diff harder (impossible?) to read.

Copy link
Author

Choose a reason for hiding this comment

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

Got it, too. I'm fixing it soon. For the time being, please resort to "Hide white space changes", which is found under "Diff settings" button on a pull-request screen.

@stathissideris
Copy link
Owner

  1. We can continue work with this PR, new commits to your branch will be included, no need for a separate PR
  2. I would like to keep backwards compatibility for the syntax, so if this feature introduces new semantics, it would need to be disabled by default and we should have a command-line option to enable it. Having it enabled by default would not work because some people use ditaa in an automated context and it may be hard for them to go and fix all invocations (and also it will create workload for them because they would have to check that everything renders the same way as before).
  3. We would also need some documentation to demonstrate this great feature too!

@dakusui
Copy link
Author

dakusui commented Oct 28, 2018

  1. Thank you for that.
  2. Understood. I'll try to implement such an option which defaults to keep compatible behaviour.
  3. I've added an action item for that and the section "LaTeX mode" in the description will be the documentation and it should be integrated in "README.md" of ditaa in this pull request. Please feel free to edit it for improvement and give me feedbacks.

@dakusui
Copy link
Author

dakusui commented Oct 28, 2018

@stathissideris , I tried hard to fix the code formatting issue, but I found it's quite a bit painful. Maybe we should reformat the entire project first and resume working on this feature if it's hard to review even if we use "Ignore white space changes" feature of GitHub.
Can you please provide a code formatting style for some IDE such as Eclipse or IntelliJ? Or any style guideline if you have?

@stathissideris
Copy link
Owner

I have stopped working with Java for many years, so I don't have a preferred style. Are you using the Intellij defaults?

@dakusui
Copy link
Author

dakusui commented Oct 28, 2018

I can use IntelliJ's default, but the one I am using daily basis is the following one which has 2 characters for indentation to save horizontal space, no tabs to be portable, etc.

https://github.com/dakusui/jcunit/tree/0.8.x-develop/src/site/resources/style

@stathissideris
Copy link
Owner

2 characters and spaces sounds like a reasonable default to me. Could you please send me a new PR on the current master that applies that to the whole project? Then you can git rebase this PR on the new master and continue work. Thanks!

@dakusui
Copy link
Author

dakusui commented Oct 28, 2018

Understood, I have opened this pull request #43 for it.

@dakusui dakusui changed the title Implement Latex math support (WIP; Please do not merge this pull request yet) Implement LaTeX math support (WIP; Please do not merge this pull request yet) Oct 28, 2018
@dakusui
Copy link
Author

dakusui commented Oct 29, 2018

I've created another branch that contains this branch's content and pull request which is directed from it to a code formatted branch.
The diff is available here. I think this is reviewable and we should create a new pull request to be merged from it eventually.

@dakusui
Copy link
Author

dakusui commented Nov 3, 2018

Please do not look into this pull request anymore. Refer to this one for review.

@dakusui dakusui closed this Nov 3, 2018
@dakusui
Copy link
Author

dakusui commented Nov 3, 2018

@stathissideris I have invited you to my forked repository as a collaborator for the review's sake. Please find an email from github or let me know to send it to you.

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.

2 participants