Fix parenthesis after links in markdown lexer #1449
Conversation
Ravlen
changed the title
pyrmont
added
author-action
|
@pyrmont Ahhhh. 😅
Ok, needs a little more work. 😁 Good catch! 🙇 FYI, Rouge dingus seems to have the best title text coloring:
In GitHub it's not great: [This is not a link] (example.com)
[link is followed](example.com) (parentheses)
[link is followed](example.com) (paren theses)
[link is followed](example.com "Title text") paren (theses)
[This link is referenced][exampleref]
[exampleref]: http://example.com "Title text"Strangely, in GitLab the Title text looks more like GitHub, even though it's using Rouge, must have selected no color for that item (
|
pyrmont
force-pushed
the
ravlen.bugfix.markdown-link-parens
branch
from
ffe07cb to
765a65f
Compare
|
@Ravlen I made a couple of changes. What do you think? |
|
@pyrmont Wow, that was a lot more work than I expected was required, sorry for the hassle! I pulled your commits and tested it now, and looks fantastic, thanks a lot! I also see my multi-line link fix is working too, glad it didn't break anything!
|
| push :url | ||
| end | ||
|
|
||
| # links and images | ||
| rule %r/(!?\[)(#{edot}*?|[^\]]*?)(\])(?=[\[(])/ do | ||
| rule %r/(!?\[)((?:\\.|[^\\\]]|`.*?`)*?)(\])(?=[\[(])/ do |
There was a problem hiding this comment.
@Ravlen This is the bit I'm unhappy about. It feels like a gross hack to allow the use of [[ and ]] for TOML headings but I couldn't see another way to do it.
There was a problem hiding this comment.
@pyrmont FYI, don't feel like you have to merge this right away, I'm just swamped at work. When I get some free time I fully intend to come back to this and see if I can come up with an alternative. 🙇
There was a problem hiding this comment.
🤔 As someone coming to this way late, I'm not quite sure what the concern is about the TOML headings. @Ravlen any insights? Is the concern it's syntactically incorrect, or the Regex is nasty?
|
@pyrmont Sorry for not replying here sooner... I've tried to come up with some recursive bracket matching algorithm that would work without need specific exceptions to make TOML work, but I just don't have the skill yet. I still hope to find the time to research this more, learn more about Rouge so I can start making better contributions, but things just have just continued to get busier. For now, I think this is the best I can hope for, and I think we can merge this to at least get it rendering well, and I'll try to revisit if work ever lets up... 🙇 |
| push :url | ||
| end | ||
|
|
||
| # links and images | ||
| rule %r/(!?\[)(#{edot}*?|[^\]]*?)(\])(?=[\[(])/ do | ||
| rule %r/(!?\[)((?:\\.|[^\\\]]|`.*?`)*?)(\])(?=[\[(])/ do |
There was a problem hiding this comment.
🤔 As someone coming to this way late, I'm not quite sure what the concern is about the TOML headings. @Ravlen any insights? Is the concern it's syntactically incorrect, or the Regex is nasty?
|
|
||
| rule %r/[ \t]+/, Text | ||
| rule %r/[ \t]+/, Text, :title |
There was a problem hiding this comment.
This was pre-existing, but I wonder why this used _\t here instead of \s which would cover all whitespace (underscore used to emphasize literal space).
|
@Ravlen @dblessing Look like this is mergeable? |
Fixes #1448
Currently, there is an odd edge case where a link followed by an open parenthesis triggers incorrect coloring: the last text should be uncolored, it's plaintext. (You can ignore the reference link in the screenshot, I was just making sure the code coloring was properly "different" for regular and referenced links):
When looking at the code, I couldn't understand why it was done as:
It seemed to make sense to just do it the same as for link references:
and remove
inline_titleandinline_url. When removed, everything seemed to work as expected:@pyrmont Could you take a look? I couldn't find any failures in the markdown sample file, but I might not be considering all the possibilities.