SassDoc / sassdoc

Release the docs!
http://sassdoc.com
MIT License
1.41k stars 56 forks source link

Parsing strings with " and ' #487

Open lazarljubenovic opened 7 years ago

lazarljubenovic commented 7 years ago

Look like it completely ignores if I chose " or ' and just looks for either one.

Source

@error "This is a string with an apostrophe. It's nice, right?";
@error 'This is a string with "quotes"';

Generated

image

KittyGiraudel commented 7 years ago

Hi. I just checked our regular expression for that and yep, it's bad: /@error\s+(?:'|")([^'"]+)/g. I'll see if we can easily fix it.

lazarljubenovic commented 7 years ago

Thanks for looking into it.

I also just noticed two other things:

lazarljubenovic commented 7 years ago

But I don't think this can be properly solved with a regex anyway. The only way to properly do this is building an AST (I was under the impression that this happens under the hood in sassdoc; didn't peek into sources yet).

Here's a quick attempt to fix the problem I originally spotted: https://regex101.com/r/I3uzCM/2

Regex: /^\s*@error\s+(?:'([^']+)|"([^"]+))/gm (note the m flag for figuring line start) Test string:

@error 'me yes yes'
@error "yiss"
@error 'me "too"'
@error "me 'too'"
@error "I'm a multiline string
        and I still work";
@error "Escaping \" a quote";

// @error "not me please"
/* @error 'nope' */
/* some stuff here
   @error 'uh not me im a comment'
*/
color: red; @error 'would be nice';
@error would be nice

I don't see how to solve other problems trivially with a regex.

KittyGiraudel commented 7 years ago
  • It doesn't catch @error Unquoted error, which is not a big deal for me since I usually write them like this only while prototyping.

This makes me think that we could in theory get everything until the next semi-colon, then strip first and last character. Right?

lazarljubenovic commented 7 years ago

Hmm... The last statement in a block doesn't need a semi-colon. Could break a lot since the errors are usually thrown inside an @if statement. The following is valid:

@if typeof $arg != number {
  @error nope
}

A quick implementation: https://regex101.com/r/EBJAsE/3

KittyGiraudel commented 7 years ago

True…

lazarljubenovic commented 7 years ago

This doesn't really solve the problem fully (as I explained, it probably needs to be more full-blown-parser-like, not a single regex), but here's an interesting thing I just learned from a talk I watched: backreferences. I was immediately reminded of this issue. https://youtu.be/EkluES9Rvak?t=2393

('|").+?\1

Basically it matches whatever was matched in the first parantheses (a capturing group). So if it was ', the \1 will refer to '. If it was ", it will refer to ".

It doesn't cover all cases we've listed above, but it surely works better than it does currently, I think. So it might be a good temporarly fix.