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

reference-links not handled like pandoc or github #31

Closed
smondet opened this issue Sep 30, 2013 · 15 comments
Closed

reference-links not handled like pandoc or github #31

smondet opened this issue Sep 30, 2013 · 15 comments
Milestone

Comments

@smondet
Copy link

smondet commented Sep 30, 2013

The test case is:

[ref]

[ref]: http://ocaml.org

With omd:

 $ printf "[ref]\n\n[ref]: http://ocaml.org\n" | omd
<p><a href='' title='Broken URL'>ref</a>: http://ocaml.org</p>

With pandoc:

 $ printf "[ref]\n\n[ref]: http://ocaml.org\n" | pandoc -r markdown_strict
<p><a href="http://ocaml.org">ref</a></p>

and with github:

ref

My omd is the one in Opam:

 $ omd -version
This is version 0.5.5 of Wed Sep 18 14:06:20 UTC 2013.
@pw374
Copy link
Contributor

pw374 commented Sep 30, 2013

Thanks for your feedback.

[ref] is not a feature of Markdown, so it's sort of "normal" that it doesn't work...

So when you write

[ref]

[ref]: http://ocaml.org

it's like writing [ref][ref] followed by this independent text : http://ocaml.org. That's why it reports a broken link, because "ref" was never defined.

If you write this instead:

[ref]: http://ocaml.org

[ref]

it returns <p>[ref]</p>, and that clearly states I have not implemented the {github, pandoc, ..} shortcut style references.

So, instead you should write this:

[ref][ref]

[ref]: http://ocaml.org

which produces

<p><a href='http://ocaml.org'>ref</a></p>

Or

[ref][]

[ref]: http://ocaml.org

which is the official shortcut style.

That being said, I agree to eventually implement this feature, maybe for omd 0.5.6 or 0.5.7 hopefully.
I do wonder if it's really a good feature to have. Maybe I'll provide a way to deactivate it (or activate it, depending on which is the default behaviour).

@pw374
Copy link
Contributor

pw374 commented Sep 30, 2013

Oh, there's a bug in github's markdown renderer...
writing this:

```
[foo]: http://bar
```

gives this:

[foo]: http://bar

(an empty rectangle... if it's not empty anymore, it means the bug has been fixed)

@smondet
Copy link
Author

smondet commented Oct 1, 2013

Thanks I did not know [ref][] was the standard (I was trusting Pandoc's markdown_strict format).

And yes having the [ref] version implemented would be useful but not critical.

@pw374
Copy link
Contributor

pw374 commented Oct 1, 2013

I suspect Pandoc's markdown_strict feature to be not very well maintained. But since I haven't made any bug reports even when I should have, I'm not (yet) blaming Pandoc's authors and/or maintainers.

I based the implementation of OMD on this document: http://daringfireball.net/projects/markdown/syntax.

However I do have a doubt about allowing more than one space character between the first part between brackets and the second part: I'm not sure why OMD allows multiple line breaks (this might be changed to allowing a single space character in the next version).

Also, (half) surprisingly, pandoc doesn't generate an html link when a reference is broken, meaning a file with just [foo][bar] will translate to <p>[foo][bar]</p>. Maybe I should do the same, instead of generating Broken URL.

@avsm
Copy link
Contributor

avsm commented Oct 1, 2013

I think passing through syntax that isn't recognized is the correct behavior, so that's preferred instead of Broken URL (although a lint mode that warns about such things might be useful)

@pw374
Copy link
Contributor

pw374 commented Oct 1, 2013

Sadly, I think you're right. That means I either have to change the type Omd.element to add fallback information for Ref and Img_ref, or make a 2-pass conversion (instead of a 1-pass), the first being for collecting reference definitions... Since it would be a nightmare to maintain a smart 2-pass conversion, it would be a naive 2-pass conversion. So, I guess I'll change Omd.element.

Let's plan this for version 0.6.0 then. (Yes, I've been trying to give more meaningful version numbers since 0.5.0.)

@ghost ghost assigned pw374 Oct 1, 2013
@pw374
Copy link
Contributor

pw374 commented Oct 1, 2013

Hmmm, it's somehow related to https://github.com/pw374/omd/issues/21 because if we have locations in the AST, they could potentially be enough to implement fallbacks...

@pw374
Copy link
Contributor

pw374 commented Oct 2, 2013

3b4babe replaces Broken URLs by a "passing-through" implemented using additional information from the AST. The parser was adapted to generate such information. (I plan on making the opam package later this week.)

pw374 added a commit that referenced this issue Oct 2, 2013
@pw374
Copy link
Contributor

pw374 commented Oct 2, 2013

57b8379 implements non-regular references such as in

[ref]

[ref]: http://ocaml.org

Also, 4e4784f prevents two-part references from having its 2 parts separated by other things than "zero, one or multiple spaces, or a newline (and not a combination of them)" (otherwise it doesn't work well with non regular references).

@pw374 pw374 closed this as completed Oct 2, 2013
@Chris00
Copy link
Contributor

Chris00 commented Nov 20, 2013

Actually, I think this feature should be opt-in only. Indeed, if it makes sense got Github the implement these kind of links because the posts are rather short, there are several places on ocaml.org where the brackets are used for something else than a link. [ref][] is the standard way and does not appear in normal text, [ref] should be enabled only in those specific situations where one knows it will not cause harm.

At some point, I think omd.mli also needs to be more precise about the "non standard" features it implements.

@Chris00
Copy link
Contributor

Chris00 commented Nov 20, 2013

Please reopen.

@pw374 pw374 reopened this Nov 20, 2013
@pw374
Copy link
Contributor

pw374 commented Nov 20, 2013

Actually, I think this feature should be opt-in only. Indeed, if it makes sense got Github the implement these kind of links because the posts are rather short, there are several places on ocaml.org where the brackets are used for something else than a link. [ref][] is the standard way and does not appear in normal text, [ref] should be enabled only in those specific situations where one knows it will not cause harm.

[ref] falls back to just [ref] when ref is not an existing reference, so it could cause harm when [ref] really means [ref] while ref is an existing reference.

At some point, I think omd.mli also needs to be more precise about the "non standard" features it implements.

I agree.

@Chris00
Copy link
Contributor

Chris00 commented Nov 20, 2013

[ref] falls back to just [ref] when ref is not an existing reference, so it could cause harm when [ref] really means [ref] while ref is an existing reference.

Except that the content is not processed (it could contain code, links,...).

@pw374
Copy link
Contributor

pw374 commented Nov 20, 2013

On 20 Nov 2013, at 13:08, Christophe Troestler [email protected] wrote:

[ref] falls back to just [ref] when ref is not an existing reference, so it could cause harm when [ref] really means [ref] while ref is an existing reference.

Except that the content is not processed (it could contain code, links,…).

Indeed. The fallback should be of type Omd.t, not string.
I guess I'll change that soon.

@pw374
Copy link
Contributor

pw374 commented Jun 9, 2014

I had forgotten about this one...

It should be fixed by a7ea3eb

@pw374 pw374 closed this as completed Jun 9, 2014
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

No branches or pull requests

4 participants