Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gfm] Prevent markdown mode from parsing formatting within vanilla links (closes #4079) #4106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0b10011
Copy link
Contributor

@0b10011 0b10011 commented Jul 10, 2016

The bug that #4079 was coming across was the fact that markdown mode parses separately from gfm mode, and isn't aware of the parsing happening in gfm mode. Additionally, either an asterisk anywhere or an underscore after a non word character had to appear in the url. That leaves a few different routes for fixing that I can think of.

First, we could make markdown mode aware of autolinking, but this is pushing more of the gfm mode logic into markdown mode that should really be avoided. Additionally, putting small hacks to prevent formatting within links would be more fragile than just building in the autolinking entirely in markdown mode (and verbose, as we'd have to put it in all of the formatting logic).

Secondly, we could give gfm mode access to markdown mode's state and have gfm mode try to manage it, but that would make refactoring states within a mode more prone to breaking overlays. So, also not ideal.

Thirdly, we could simply give the gfm mode's tokens priority, which would fix the link, but everything after it would still be in a different state than it should be (italicized when it shouldn't be).

Lastly, we can force markdown to freeze state until after the link, which is ideal and the route I ended up taking. That is, the state before the link in markdown should be exactly the same as after the link. The simplest way I could think of to achieve this was to add a freezeBaseState flag to gfm mode that the overlay.js code would look for and skip base mode parsing when it was encountered. It would then set the base position to the overlay position so they both start parsing again from the same spot (after the link).

@marijnh
Copy link
Member

marijnh commented Jul 11, 2016

This seems to be pulling the mode/overlay addon in the direction of the mode/multiplex one.

Do you, at a glance, know any compelling reason why the GFM mode shouldn't be implemented through hooks provided by the Markdown mode, the way the clike and sql dialects work? A lot of its complexity seems to come from overlays just beeing a rather bad fit for such an interwoven set of modes.

@0b10011
Copy link
Contributor Author

0b10011 commented Jul 11, 2016

This seems to be pulling the mode/overlay addon in the direction of the mode/multiplex one.

Yeah, I think you're right. I was thinking the multiplex addon relied more on the base mode being aware of the overlay mode.

Do you, at a glance, know any compelling reason why the GFM mode shouldn't be implemented through hooks provided by the Markdown mode, the way the clike and sql dialects work?

Hmm, that might be an option. It would require a general token hook to work with URLs, SHAs, etc.

Is it possible to extend modes defined in this way, though? That is, if gfm mode was defined using hooks instead, would someone else be able to come along and extend gfm mode, adding their own hooks? Or would they have to manually copy and paste the hooks into their own mode? If there's no way to extend, I think that would be a reason not to.

A lot of its complexity seems to come from overlays just beeing a rather bad fit for such an interwoven set of modes.

Yeah, I agree. I can't remember what made me choose the overlay addon 4 years ago, but with how it functions currently, it just isn't a good fit. Hooks may be the way to go. I'll look into it and get back to you. Thanks for the direction!

@marijnh
Copy link
Member

marijnh commented Jul 11, 2016

Is it possible to extend modes defined in this way, though?

The idea is to provide configuration parameters and/or callbacks that allow all aspects of the language that might change between dialects to be customized. What this looks like will differ per language, and you'll probably not be able to (or want to) cover all possible dialects, but yes, the experiences with the clike, css, and sql modes has been that this is a reasonable way to do language variations -- it's definitely easier to extend than the current GFM mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants