prosemirror/prosemirror-markdown

Fork 0

Unnecessary escape for the period (.) character #101

New issue

Closed

opened 2023-07-12 20:43:38 +02:00 by dragonman225 · 3 comments

dragonman225 commented

2023-07-12 20:43:38 +02:00

(Migrated from github.com)

Pasting the following Markdown to the example, switch to WYSIWYG and switch back to Markdown,

123 [example.com](https://example.com)

123 [0.com](https://example.com)

123 [example.1](https://example.com)

123 [2.2](https://example.com)

4.4

* 5.5
* 6\. 6
7. 7\. 7

8\. 8

it becomes

123 [example.com](https://example.com)

123 [0\.com](https://example.com)

123 [example.1](https://example.com)

123 [2\.2](https://example.com)

4\.4

* 5\.5
* 6\. 6

7. 7\. 7

8\. 8

As far as I know, the periods in 0.com, 2.2, 4.4, and 5.5 don't need to be escaped, and escaping them makes the generated Markdown less readable.

Pasting the following Markdown to the [example](https://prosemirror.net/examples/markdown/), switch to WYSIWYG and switch back to Markdown, ```md 123 [example.com](https://example.com) 123 [0.com](https://example.com) 123 [example.1](https://example.com) 123 [2.2](https://example.com) 4.4 * 5.5 * 6\. 6 7. 7\. 7 8\. 8 ``` it becomes ```md 123 [example.com](https://example.com) 123 [0\.com](https://example.com) 123 [example.1](https://example.com) 123 [2\.2](https://example.com) 4\.4 * 5\.5 * 6\. 6 7. 7\. 7 8\. 8 ``` As far as I know, the periods in `0.com`, `2.2`, `4.4`, and `5.5` don't need to be escaped, and escaping them makes the generated Markdown less readable.

dragonman225 commented

2023-07-12 21:13:48 +02:00

(Migrated from github.com)

I looked into the issue and found it's related to this commit and the fix for #75

I also attached a patch and it consists of two parts.

The first part is to set this.atBlockStart = true only when the serializer is really writing to block start in renderInline(). (In current code, every text node in a block is rendered with this.atBlockStart = true.) This fixes periods in link text (0.com, 2.2).

The second part is to replace text that would actually be parsed as a list item in esc(). For example, 4.4 would not be parsed as a list item, while 4. 4 would. So I add a space character (\s) in the regex.

I looked into the issue and found it's related to [this commit](https://github.com/ProseMirror/prosemirror-markdown/commit/b6552842c298372ac8087ab398ce1e24ad734ec6) and the fix for #75 I also attached a patch and it consists of two parts. The first part is to set `this.atBlockStart = true` only when the serializer is really writing to block start in `renderInline()`. (In current code, every text node in a block is rendered with `this.atBlockStart = true`.) This fixes periods in link text (`0.com`, `2.2`). The second part is to replace text that would actually be parsed as a list item in `esc()`. For example, `4.4` would not be parsed as a list item, while `4. 4` would. So I add a space character (`\s`) in the regex.

marijnh commented

2023-07-14 17:10:23 +02:00

(Migrated from github.com)

Please take a look at 8f0a84fdcb and let me know if that also looks like a solution to the atBlockStart problem.

Please take a look at 8f0a84fdcb16c4a544f5bbcbe41e67c203a571b6 and let me know if that also looks like a solution to the `atBlockStart` problem.

dragonman225 commented

2023-07-14 23:49:21 +02:00

(Migrated from github.com)

Thank you for the suggestion. The code of 8f0a84f makes sense to me. I tried and it also worked well.

Though I'd like to suggest adding spaces after the periods in this test:

it("does not escape list markers in the middle of paragraphs", () => {
  same("123 [0.com](foo)\n\n123 [2.2](foo)",
       doc(p("123 ", a("0.com")), p("123 ", a("2.2"))))
})

e.g.

it("does not escape list markers in the middle of paragraphs", () => {
  same("123 [0. com](foo)\n\n123 [2. 2](foo)",
       doc(p("123 ", a("0. com")), p("123 ", a("2. 2"))))
})

Because after we changed the regex in esc(), without spaces the periods would not be escaped, regardless of whether atBlockStart is correct.

Thank you for the suggestion. The code of 8f0a84f makes sense to me. I tried and it also worked well. Though I'd like to suggest adding spaces after the periods in this test: ```js it("does not escape list markers in the middle of paragraphs", () => { same("123 [0.com](foo)\n\n123 [2.2](foo)", doc(p("123 ", a("0.com")), p("123 ", a("2.2")))) }) ``` e.g. ```js it("does not escape list markers in the middle of paragraphs", () => { same("123 [0. com](foo)\n\n123 [2. 2](foo)", doc(p("123 ", a("0. com")), p("123 ", a("2. 2")))) }) ``` Because after we changed the regex in `esc()`, without spaces the periods would not be escaped, regardless of whether `atBlockStart` is correct.

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

prosemirror/prosemirror-markdown#101

No description provided.

Rows
Columns