search

Lukasa / language-restructuredtext

A ReStructuredText syntax package for Atom
MIT License
27 stars 15 forks source link

More conservative link matching. #57

Closed phyllisstein closed 6 years ago

phyllisstein commented 6 years ago

Kind of an edge case, but including TypeScript snippets with tagged template literals previously broke the highlighting, causing everything in the file after the closing ` to be treated as a link title. This slightly more conservative match seems to cover the most common way links are used without matching them promiscuously across lines.

Before

before

After

after

phyllisstein commented 6 years ago

Also added some changes to add TypeScript to the embedded syntaxes and fix the end-of-block matching. To the best of my knowledge, using \1 in the end regex would only create a backref to that expression; there's no way to reach through to recover the begin pattern's matches. The change in 5795ddd is, comparatively, way less clever and more naïve, but it works:

Before

before

After

after

Alhadis commented 6 years ago

Looks good. Just one small thing:

To the best of my knowledge, using \1 in the end regex would only create a backref to that expression; there's no way to reach through to recover the begin pattern's matches

There is, and that's how the TextMate highlighting system implements features like heredocs and Markdown code-fences. Sharing of capturing groups is somewhat of a "magic feature" that's limited to begin/end pairs only.

The capture it's back-referencing is the leading indentation of the beginning match; which is how we're able to nest one indented component inside a larger indented component.

But everything else looks good!

phyllisstein commented 6 years ago

Gotcha! TMYK🌠. Shall I back that commit out, then, and play with the regexes a bit more? It definitely wasn't matching as expected, but that's arguably a problem for another PR.

Alhadis commented 6 years ago

Probably better to leave it for a different PR, I'd say.

phyllisstein commented 6 years ago

👍 Duly dropped! I'll see if I can sort through the regex issue this weekend. Thanks for the feedback!

Alhadis commented 6 years ago

I'll hold off on cutting a release until you get back to me. =)

Alhadis commented 6 years ago

Hey, @phyllisstein, how did you go with tackling the other half of this PR?

I'd like to get another release cut sometime soon-ish, and it'd be great if we could get this fix on-board too.

phyllisstein commented 6 years ago

I'm so sorry for not getting back to you sooner. I'm actually not able to reproduce the issue with end captures in a recent build of Atom---could it be that they fixed something on their end while I was lollygagging?

screenshot 1