43081j / eslint-plugin-lit

lit-html support for ESLint
120 stars 22 forks source link

highlight escape sequence rather than quasi #103

Closed 43081j closed 3 years ago

43081j commented 3 years ago

Fixes #101 Fixes #106

@stramel would you mind double checking this in an editor? we need to be sure that +1 is right again 😂

i think it is...

html`foo`

// quasi.range[0] would be the first backtick (`)
// lets say we match `/o/`, the first regex index will be 1 i suppose
// so range[0] + 1 would give us the `f` but we want the `o`, so we +1 to account for the backtick
stramel commented 3 years ago

@43081j Doesn't quite seem to be working? image

Not getting any errors either when using an invalid test case 😬

return html` foo \\0123 bar `;

image

Using:

return html` foo \0123 bar `;

image

43081j commented 3 years ago

i've added a fixer now too

i checked it in my editor and it seems like it lines up. how about for you?

stramel commented 3 years ago

Getting really close! A space after is still highlighted.

image

With no space after, image

Fix seems to be working: image

43081j commented 3 years ago

should all be sorted now 👍

43081j commented 3 years ago

think thats sorted now @stramel

43081j commented 3 years ago

what makes you think that is off?

\\\0123 should underline \0123 as its an unescaped sequence. its meant to underline the backslash inside the sequence

the regex seems to be doing what i intended it to do:

(^|[^\\](?:\\\\)*) is group 1 which matches the prefix (\\([1-7][0-7]*|[0-7]*|[0-7]{2,})) is group 2 which matches the unescaped sequence, and also contains group 3 which we don't use

stramel commented 3 years ago

Because your regex also if given foobar\\\0123 matches on r\\\0123. Another reason I think it's off is because it seems that you're highlighting group 2 currently. Posted a regexr link above

43081j commented 3 years ago

i am highlighting group 2, on purpose as its the unescaped escape sequence.

foo \0123

1 = ` 2 =\0123 3 =0123` (but doesn't matter, isn't used)

this is the intended result so im a bit lost.

foobar\\\0123

1 = r\\ 2 = \0123

so we highlight the unescaped sequence, which is \0123 as shown in your screenshot

stramel commented 3 years ago

So I guess what I'm arguing is that the entire string including the redundant \s be highlighted as well so \0123 and \\\0123 would both be fully highlighted.

foo \0123

1 = ` 2 =\0123 3 =0123` (but doesn't matter, isn't used)

This was the result of the match

// match
[
  ' \\0123',
  ' ',
  '\\0123',
  '0123',
  index: 3,
  input: 'foo \\0123 bar',
  groups: undefined
]

foobar\\\0123

1 = r\\ 2 = \0123

This was the result of the match

//match
[
  'r\\\\\\0123',
  'r\\\\',
  '\\0123',
  '0123',
  index: 5,
  input: 'foobar\\\\\\0123 bar',
  groups: undefined
]