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

Adds refered links to Markdown #3686

Closed

Conversation

akwiatkowski
Copy link

No description provided.

@@ -641,3 +710,5 @@ class Markdown::Parser
end
end
end

0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray character? :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, fixing it now! :)

@akwiatkowski
Copy link
Author

@Sija updated

@asterite
Copy link
Member

I think it's kind of OK, there are many regexes and subtitutions, though, but I don't think the performance difference will matter. I'm not sure about replacing [...] with [...](...) and later processing things. I'll review it more thoroughly later.

@spalladino
Copy link
Contributor

Maybe you can remove the 2nd pass inprocess_for_references by converting references along with the rest of the doc? As for the first pass for gathering refs, I'm not sure it can be avoided.

@akwiatkowski
Copy link
Author

@spalladino I'm not sure if I understand you.

I had an attempt before to process referenced links ([Name][label]) to html code (<a href="example.com">Name</a>) in first pass but there was problem with mixing other markdown "tags", for example [Name **with strong**][label].

I could remove 2nd pass if there would exist parse method to just parse Name **with strong**. Current document parsing style is different and I have strong feeling against running parser within parser.

I'm more than ok with future optimization, but at this moment I don't think if I could make it faster, more markdown spec accurate and easily to maintain.

@asterite
Copy link
Member

I think it's OK if we merge this. Eventually we should rewrite the whole parser with a better overall algorithm, maybe taking a look at how other parsers are implemented, or checking if the official site has some pointers on that.

@akwiatkowski
Copy link
Author

@asterite I've checked very briefly and it was not enough. I'm writing my own Jekyll-like blog generator so markdown will be quite important to me. I'll work on it later.

@ysbaddaden
Copy link
Contributor

@akwiatkowski you'll may like https://github.com/ysbaddaden/crystal-cmark for the time being?

@miketheman
Copy link
Contributor

#codetriage
Looking at the evolution here, there seems to be a couple of questions we should try to answer:

  1. What spec does the stdlib markdown attempt to comply with? The Daring Fireball spec appears to state that these are meant to work. I recognize that it's not a full spec, and that's why CommonMark exists.
  2. While the CommonMark lib is a viable alternative (:clap: @ysbaddaden), it is not the built-in for the language, and requires an external C library. While this may ultimately perform better, it is not common (pun!) to have languages embed the Markdown parser, rather rely on a pure-language implementation of the parser/renderer as external libraries. This has historically led to multiple implementations, with slightly different behaviors and semantics, so having The Right Thing in the language's stdlib goes a long way to reducing friction. Would having a pure Crystal implementation of the CommonMark spec in stdlib be the right way to go, and if so, how receptive are we to this kind of overhaul?

(incidentally, I wrote the link above with a reference link syntax, thanks GitHub editor!)

@ysbaddaden
Copy link
Contributor

ysbaddaden commented Apr 21, 2017

Would having a pure Crystal implementation of the CommonMark spec in stdlib be the right way to go?

Yes, definitely.

The cmark shard exists because I needed a complete Markdown implementation, cmark is the reference implementation of CommonMark, and it only took minutes to write and publish bindings for it. But ultimately, a pure crystal, complete implementation of Markdow, following the CommonMark spec is the way to go.

@asterite
Copy link
Member

I think it would be wise to port the CommonMark implementation, or just write an implementation that conforms to CommonMark, and replace the current one. The current one was just created so that docs can be generated and shown without a new dependency, but it's far from complete, and it probably has some bugs too.

@RX14
Copy link
Contributor

RX14 commented Apr 21, 2017

Do we really need a markdown parser in the stdlib? I realise that it's required by the crystal compiler but if that's the only reason, wouldn't it be better to develop ways for the compiler to be able to use external shards?

@asterite
Copy link
Member

Since we'll use markd in the future, which implements this, we can close this.

@asterite asterite closed this Sep 29, 2017
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.

None yet

7 participants