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

stringify: incorrect entity encoding #498

Closed
ysiw opened this issue May 28, 2020 · 10 comments
Closed

stringify: incorrect entity encoding #498

ysiw opened this issue May 28, 2020 · 10 comments
Labels
🗄 area/interface This affects the public interface 🙅 no/wontfix This is not (enough of) an issue for this project remark-stringify 🐛 type/bug This is a problem

Comments

@ysiw
Copy link

ysiw commented May 28, 2020

remark-stringify outputs incorrect escaped entites in table if options.entities is "escape"

$ cat a.md
| aaa        | aaa |
| ---------- | --- |
| http://abc |     |
$ remark a.md
| aaa             | aaa |
| --------------- | --- |
| http://abc |     |
a.md: no issues found
$ remark --setting '"entities":"escape"' a.md
| aaa                 | aaa |
| ------------------- | --- |
| http://abc |     |
a.md: no issues found
$
@ysiw ysiw added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels May 28, 2020
@wooorm
Copy link
Member

wooorm commented May 30, 2020

What is the reason for using entities: 'escape'?

@wooorm
Copy link
Member

wooorm commented May 30, 2020

This is indeed a bug. Double escaping is going on if entities: 'escape' is defined.
I think it makes the most sense to remove the option altogether.

@ysiw
Copy link
Author

ysiw commented May 30, 2020

I prefer some entites are escaped, such as ✓ (✓), ✗ (✗). It is the only way to do it. entities: true is not an option for me, because it escapes all CJK characters.

@wooorm
Copy link
Member

wooorm commented Jun 9, 2020

I prefer some entites are escaped, such as ✓ (✓), ✗ (✗)

That behavior is closer to entities: true than entities: 'escape'.
escape used to work but is now broken, and should do the inverse: https://github.com/wooorm/stringify-entities/tree/355100188e1f3661359e7404115cd7e04c75ed41#optionsescapeonly.

I think removing 'escape' makes the most sense.

@ysiw
Copy link
Author

ysiw commented Jun 12, 2020

I suggest modifing behavior of entites: 'escape' to escape ONLY all character entites listed in https://dev.w3.org/html5/html-author/charref

There is a package character-entities which has the complete list.

Current implementation gives me no option to keep my document unchanged.

image

@wooorm
Copy link
Member

wooorm commented Jun 12, 2020

I suggest modifing behavior of entites: 'escape' to escape ONLY all character entites listed in https://dev.w3.org/html5/html-author/charref

That does include many ASCII / other characters, such as a space, a tab, a line feed, +, -, :, and so many more!


As you are already invested in Unicode, with Chinese characters, I don’t really get why check and cross can’t be in unicode either?

I instead strongly think we should drop options on encoding character references. /CC @ChristianMurphy, what do you think?

@ChristianMurphy
Copy link
Member

drop options on encoding character references

As in remove options.entities from the project entirely?

@wooorm
Copy link
Member

wooorm commented Jun 12, 2020

Yes

@wooorm
Copy link
Member

wooorm commented Jun 12, 2020

background: it was added because I recently made stringify-entities, which had those options. Now, years later, we definitely live in a unicode world, there is basically no reason anymore to use character references.

@ChristianMurphy
Copy link
Member

Removing the option works for me.
The only character entity that I've used in projects recently is  , which I believe has a Unicode equivalent.

@wooorm wooorm changed the title incorrect escaped entities in table stringify: incorrect entity encoding Jul 17, 2020
@wooorm wooorm added remark-stringify 🐛 type/bug This is a problem 🗄 area/interface This affects the public interface 🙅 no/wontfix This is not (enough of) an issue for this project and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Jul 17, 2020
@wooorm wooorm closed this as completed in 4a49fbc Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 🙅 no/wontfix This is not (enough of) an issue for this project remark-stringify 🐛 type/bug This is a problem
Development

No branches or pull requests

3 participants