UI: Add workaround to exclude emoji from task escaping #7813
Conversation
backspace
force-pushed
the
b-ui/exec-emoji-task-name
branch
from
e9abc5a
to
01746ed
Compare
Ember Asset Size actionAs of c3e55c1 Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
|
If I'm reading this correctly, the underlying issue is that If that's the case you can use the regex str.replace(/[^a-zA-Z0-9,._+@%/-]/g, '\\$&');
// "emoji\�\�"
str.replace(/[^a-zA-Z0-9,._+@%/-]/gu, '\\$&');
// "emoji\🥳"It looks like the emoji still match and get escaped but maybe that can be worked around without bringing in a dependency? |
This closes #7459. While emoji don’t actually need escaping, expanding the expression that enumerates all task name characters that don’t need escaping to include emoji is prohibitive, since it’s a discontinuous range. The emoji-regex project has such an expression and it’s 12kB. This fixes the regular expression to property escape emoji as a single character instead of as its component bytes. Thanks to @DingoEatingFuzz for the suggestion.
backspace
force-pushed
the
b-ui/exec-emoji-task-name
branch
from
1cbd75f
to
c3e55c1
Compare
|
hah WELL that’s way better, thank you, TIL! I’ve overwritten with a much smaller commit 🤓 |
| @@ -6,5 +6,6 @@ module('Unit | Utility | escape-task-name', function() { | |||
| assert.equal(escapeTaskName('plain'), 'plain'); | |||
| assert.equal(escapeTaskName('a space'), 'a\\ space'); | |||
| assert.equal(escapeTaskName('dollar $ign'), 'dollar\\ \\$ign'); | |||
| assert.equal(escapeTaskName('emoji🥳'), 'emoji\\🥳'); | |||
There was a problem hiding this comment.
Should emoji be escaped? Maybe it's harmless?
There was a problem hiding this comment.
They don’t need to be but I can’t think of a way to exclude them from escaping while still escaping things that should be, at least without something like emoji-regex. I do believe it’s harmless to escape them: slightly unsightly, but emoji in task names are surely rare 🤔
The current approach to escaping a task name is “escape everything but these known-to-be-safe characters”. My previous implementation used emoji-regex to let emoji be treated as known-to-be-safe but it seems maybe overkill when escaping them this way doesn’t add much noise. The Stack Overflow answer that I linked to for the original regular expression has a more naïve suggestion to instead escape every character but that seems hideous to me 😆
There was a problem hiding this comment.
Haha, I agree with you that escaping everything is a nope. Since this is safe I'm happy with it 😄
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This closes #7459.
While emoji don’t actually need escaping, expanding the
expression that enumerates all task name characters that
don’t need escaping to include emoji is prohibitive, since
it’s a discontinuous range. The emoji-regex project has
such an expression and it’s 12kB.
This fixes the regular expression to property escape emoji
as a single character instead of as its component bytes.
Thanks to @DingoEatingFuzz for the suggestion.