Adding Tex Inline support for the issue #386 #397

DevSerendipity · 2022-02-23T09:43:27Z

How it looks like

The inputs
A$B$C
$Hello$, look at this $\frac{x}{2}$, nice stuff!
The Outputs
\text{A}B\text{C}
Hello\text{, look at this }\frac{x}{2}\text{, nice stuff!}

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java

Zabuzard

You should also check if you can adjust the description of the command so that users know that they can now also use inline-latex.

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java

Zabuzard

Well done 👍

Tais993

subjective, so just think about it

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java

Heatmanofurioso · 2022-02-28T10:04:45Z

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java

+    }
+
+    private boolean isInvalidInlineFormat(String latex) {
+        return latex.chars().filter(ch -> ch == '$').count() % 2 == 1;


Same here.
We should escape the possible escaped \$ characters

We discussed this and agreed that we can ignore it for now.

So unless you have a cheap and easy suggestion to adjust the code to support it, we can also go without it for now.

Sorry about that.
Didn't know it was discussed.

But, wouldn't (?<!\\)\$ be a good regex for it? It would only match unescaped dollar signs

Didn't know it was discussed.

No worries, not your fault. It was probably in Discord and not here.

But, wouldn't (?<!\)$ be a good regex for it? It would only match unescaped dollar signs

Good idea. Does Javas regex engine support that? @IslamSakrak could you try that out?

We could also add a really simple unit test to test it too.

Yes, totally correct. We still have a separate GH issue open for adding tests to /tex, so I didnt want to bloat up this PR with more tasks than necessary (poor @IslamSakrak )

I actually think that, as soon as this is merged, someone should tackle the unit tests for /tex. It would benefit a lot from it.

You're correct.
I'm gonna mention this point on that issue, so it doesn't fall into the forgotten realm when those tests get addressed

I wouldn't mind testing this also, I have written some tests already. Also, first time writing tests so might help me get better at it

If you want, feel free to either create a second PR or also do it directly in this, thanks. (Assign yourself to the GH issue then though)

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java

DevSerendipity · 2022-02-28T12:52:30Z

61 conversations personal best 😭

github-actions · 2022-03-31T07:10:00Z

This pull request is stale because it has been open 30 days with no activity. Remove stale label, comment or add the valid label or this will be closed in 5 days.

Zabuzard · 2022-04-06T07:25:06Z

rip. @IslamSakrak

DevSerendipity requested review from a team as code owners February 23, 2022 09:43

Zabuzard requested changes Feb 23, 2022

View reviewed changes

Tais993 requested changes Feb 23, 2022

View reviewed changes

Zabuzard reviewed Feb 24, 2022

View reviewed changes

Zabuzard requested changes Feb 24, 2022

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java Outdated Show resolved Hide resolved

Zabuzard requested changes Feb 24, 2022

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java Outdated Show resolved Hide resolved

Zabuzard requested changes Feb 24, 2022

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java Outdated Show resolved Hide resolved

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java Outdated Show resolved Hide resolved

Zabuzard previously approved these changes Feb 24, 2022

View reviewed changes

DevSerendipity marked this pull request as draft February 24, 2022 19:11

Tais993 reviewed Feb 24, 2022

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java Outdated Show resolved Hide resolved

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java Outdated Show resolved Hide resolved

DevSerendipity dismissed Zabuzard’s stale review via ce6ce7e February 25, 2022 06:49

DevSerendipity marked this pull request as ready for review February 25, 2022 07:08

Zabuzard assigned DevSerendipity Feb 25, 2022

Zabuzard linked an issue Feb 25, 2022 that may be closed by this pull request

/tex should support inline TeX #386

Closed

Zabuzard added enhance command Modify or improve an existing command or group of commands of the bot priority: normal labels Feb 25, 2022

Zabuzard added this to the Improvement phase 1 milestone Feb 25, 2022

Tais993 previously approved these changes Feb 25, 2022

View reviewed changes

DevSerendipity dismissed Tais993’s stale review via 19546e2 February 26, 2022 06:30

Zabuzard requested changes Feb 27, 2022

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/mathcommands/TeXCommand.java Outdated Show resolved Hide resolved

Zabuzard requested changes Feb 27, 2022

View reviewed changes

DevSerendipity force-pushed the feature/tex-inline branch from f7d5ca9 to 7d031ca Compare February 28, 2022 09:53

Zabuzard previously approved these changes Feb 28, 2022

View reviewed changes

DevSerendipity dismissed Zabuzard’s stale review via 4d6da42 February 28, 2022 10:02

DevSerendipity force-pushed the feature/tex-inline branch from 4d6da42 to 90b7b94 Compare February 28, 2022 10:02

Heatmanofurioso requested changes Feb 28, 2022

View reviewed changes

Heatmanofurioso mentioned this pull request Feb 28, 2022

Add unit tests for the tex command #214

Closed

DevSerendipity added 2 commits February 28, 2022 13:45

Adding Tex Inline support for the issue Together-Java#386

9c97433

Refactoring TexCommand

8cf0a9c

DevSerendipity force-pushed the feature/tex-inline branch from 90b7b94 to 8cf0a9c Compare February 28, 2022 12:48

github-actions bot added stale inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later labels Mar 31, 2022

github-actions bot closed this Apr 6, 2022

Zabuzard mentioned this pull request May 4, 2022

Adding Tex inline support #441

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Tex Inline support for the issue #386 #397

Adding Tex Inline support for the issue #386 #397

DevSerendipity commented Feb 23, 2022

Zabuzard left a comment

Zabuzard left a comment

Tais993 left a comment

Heatmanofurioso Feb 28, 2022

Zabuzard Feb 28, 2022

Heatmanofurioso Feb 28, 2022

Zabuzard Feb 28, 2022

Zabuzard Feb 28, 2022

Zabuzard Feb 28, 2022

Zabuzard Feb 28, 2022

Heatmanofurioso Feb 28, 2022

DevSerendipity Feb 28, 2022

Zabuzard Feb 28, 2022 •

edited

Loading

DevSerendipity commented Feb 28, 2022

github-actions bot commented Mar 31, 2022

Zabuzard commented Apr 6, 2022

Adding Tex Inline support for the issue #386 #397

Adding Tex Inline support for the issue #386 #397

Conversation

DevSerendipity commented Feb 23, 2022

Zabuzard left a comment

Choose a reason for hiding this comment

Zabuzard left a comment

Choose a reason for hiding this comment

Tais993 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zabuzard Feb 28, 2022 • edited Loading

Choose a reason for hiding this comment

DevSerendipity commented Feb 28, 2022

github-actions bot commented Mar 31, 2022

Zabuzard commented Apr 6, 2022

Zabuzard Feb 28, 2022 •

edited

Loading