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

Fixing Dollar Sign Escape #4363

Closed

Conversation

Algorithm5838
Copy link
Contributor

@Algorithm5838 Algorithm5838 commented Mar 21, 2024

Improve the implementation of the dollar sign escape to address the issues with LaTeX.
Update: Done and ready for merge.

Copy link

vercel bot commented Mar 21, 2024

@Algorithm5838 is attempting to deploy a commit to the NextChat Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

Your build has completed!

Preview deployment

@daiaji
Copy link

daiaji commented Mar 21, 2024

quantizor/markdown-to-jsx#485 (comment)
I wonder if it would be better to switch to markdown-to-jsx, perhaps to make the code more lightweight. The markdown-to-jsx example recommends some code to directly define the scope in which LaTeX rendering takes effect, which looks more elegant than the symbolic substitution used today.

@Algorithm5838
Copy link
Contributor Author

Unfortunately, I am unfamiliar with it, maybe others can help out. I just fixed the inconsistent escaping.

Copy link
Contributor

@H0llyW00dzZ H0llyW00dzZ left a comment

Choose a reason for hiding this comment

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

@Algorithm5838 Just letting you know


return escapedText;
if (!isInCodeBlock) {
return line.replace(/(?<!`.*|\\)\$\d+([,.](\d+[,.])?\d+)?(?!.*\$\B)(?!.*`)/g, '\\$&');
Copy link
Contributor

Choose a reason for hiding this comment

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

you may need to handle inside single backtick, table as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your input. It does handle inside single backticks, unless one used a backtick in one line and another in the next line. Also, it does handle tables as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this?

function escapeDollarNumber(text: string): string {
  let isInCode = false;
  return text.split('\n').map(line => {
    if (line.trim() === '```' || line.trim() === '`') {
      isInCode = !isInCode;
      return line;
    }
    if (!isInCode) {
      return line.replace(/(?<!`.*|\\)\$\d+([,.](\d+[,.])?\d+)?(?!.*\$\B)(?!.*`)/g, '\\$&');
    } else {
      return line;
    }
  }).join('\n');
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for your input. It does handle inside single backticks, unless one used a backtick in one line and another in the next line. Also, it does handle tables as well.

However, it still doesn't handle properly in your deployment

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. I will try to fix it.
Thank you.

Copy link
Contributor

@H0llyW00dzZ H0llyW00dzZ left a comment

Choose a reason for hiding this comment

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

return line;
}
if (!isInCode) {
return line.replace(/(?<!\\)\$\d+([,.](\d+[,.])?\d+)?(?!.*\$\B)(?!`+)/g, '\\$&');
if (!isInMultilineCode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it's better

image

I think it should be fine for inside a table

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just made another edits. The previous one had issues rendering latex. Check the new one. Hopefully there are no more issues.

Copy link
Contributor

@H0llyW00dzZ H0llyW00dzZ left a comment

Choose a reason for hiding this comment

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

return inlineCode;
if (!isInBlockCode) {
return line.split(/(`.*?`)/g).map((segment, index) => {
if (index % 2 === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

now it only handle correctly for left but not the right one

image

@daiaji
Copy link

daiaji commented Mar 23, 2024

😄

Copy link
Contributor

@H0llyW00dzZ H0llyW00dzZ left a comment

Choose a reason for hiding this comment

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

@@ -109,7 +109,7 @@ function escapeDollarNumber(text: string): string {
if (!isInBlockCode) {
return line.split(/(`.*?`)/g).map((segment, index) => {
if (index % 2 === 0) {
return segment.replace(/(?<!\\)\$\d+([,.](\d+[,.])?\d+)?(?!.*\$\B)/g, '\\$&');
return segment.replace(/(?<!\\)\$\d+([,.](\d+[,.])?\d+)?(?!.*\$\.)/g, '\\$&');
Copy link
Contributor

Choose a reason for hiding this comment

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

better

image

Copy link
Contributor Author

@Algorithm5838 Algorithm5838 Mar 23, 2024

Choose a reason for hiding this comment

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

Unfortunately, there is a new issue with: $1$, or any other number. It always escapes numbers. I will look into it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, there is a new issue with: $1$, or any other number. It always escapes numbers. I will look into it.

@Algorithm5838 you right

image

here is the pattern for a test

$1$ $1$
$1 $2 $3 $3
`$1` $2 $3 `$4`
$x$ $hello world$  $x$  $1 $2 $3 $4
$x$ $hello world$  $x$  $1 $2 $3 $4 $x$ $hello world$  $x$
$1 $2 $3 $4 $x$
$1 2 $3 $4 $5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@Dean-YZG Dean-YZG closed this Mar 26, 2024
@Algorithm5838 Algorithm5838 deleted the fix-dollar-escape branch March 26, 2024 16:38
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.

4 participants