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

Escape - in MarkdownV2 escape regex #1

Merged
merged 1 commit into from
Jul 17, 2020
Merged

Escape - in MarkdownV2 escape regex #1

merged 1 commit into from
Jul 17, 2020

Conversation

kidonng
Copy link
Contributor

@kidonng kidonng commented Jul 17, 2020

Without escaping, the regex matches numbers as well.

EdJoPaTo added a commit that referenced this pull request Jul 17, 2020
@EdJoPaTo EdJoPaTo merged commit 7a39701 into EdJoPaTo:master Jul 17, 2020
@EdJoPaTo
Copy link
Owner

Uh... that's an ugly regex one. Thanks for pointing that out and for providing a PR!

@kidonng
Copy link
Contributor Author

kidonng commented Jul 17, 2020

By the way, can you also add an extra escape function to MarkdownV2, which is more useful than the existing one?

https://stackoverflow.com/a/60145565 (needs slight modification)

const escape = (text: string) =>
  text.replace(/(\[[^\][]*]\(http[^()]*\))|[[\]()>#+\-=|{}.!]/gi, (x, y) =>
    y ? y : '\\' + x
  )

@EdJoPaTo
Copy link
Owner

The current idea is that you do not know the input as its from a user for example. But it should be exactly displayed the way the user defined it. If the users firstname contains a [ it should be within the visible result. Even if the user first name contains bla it should be displayed exactly like that as its unknown content.
At least that's what i understand currently as escaping: Make sure the input is displayed as it is just with the markup.

If you know your input you dont need to escape it in the first place. Or only the relevant parts like this:

format.url(format.escape(label), url))

Or do i misunderstand something here? (If there is a feature request lets discuss this in its own issue so the topic is clear :) )

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.

None yet

2 participants