mixu / markdown-styles

Markdown to static HTML generator and multiple CSS themes for Markdown
1.85k stars 250 forks source link

Links to internal anchors (or implicit anchors) cause error #45

Closed nazrhyn closed 8 years ago

nazrhyn commented 8 years ago

In convert-md.js, when the renderer.link function is called on a link that looks like this:

1. [Connections](#connections)

Its call to url.parse produces something like this (assigned to parsed):

{
  protocol: null,
  slashes: null,
  auth: null,
  host: null,
  port: null,
  hostname: null,
  hash: '#connections',
  search: null,
  query: null,
  pathname: null,
  path: null,
  href: '#connections'
}

parsed.pathname is then passed to path.extname which fails with the following because pathname is null.

TypeError: Path must be a string. Received null

I use links like that to link to the implicit anchors provided by the ids generated for heading elements. Changing line 58 to...

if (!parsed.protocol && parsed.pathname) {

...fixes the problem. Not sure if that's more shotgun than you'd want, though.

a7madgamal commented 8 years ago

+1 i fixed it with var ext = path.extname(parsed.pathname || ''); should i send a pr with this fix?

a7madgamal commented 8 years ago

I hope someone can comment on this and hopefully push a fix. it's almost blocking for us now!

nazrhyn commented 8 years ago

I'd be happy to do a PR to resolve this, I just need some interaction to let me know what the maintainers would like.

mixu commented 8 years ago

@nazrhyn sorry for the delay! A PR would be great!

mixu commented 8 years ago

@nazrhyn I went ahead and fixed this, thanks @a7madgamal fir reporting and sorry for the delay again!

Published as v3.1.8

a7madgamal commented 8 years ago

thx all <3

nazrhyn commented 8 years ago

@mixu I will say that I think that doing the || '' is a bit disingenuous. It is known, by the time url.parse is called, that there is no pathname. There's no reason to even execute path.extname or run the if statement below that.

/shrug

But that's just a stylistic point. Thanks for the fix!

a7madgamal commented 8 years ago

tbh I didn't look much into the code. I just tried to fix an error that was blocking a build and that's why I didn't send a pr with that hack :)

a7madgamal commented 8 years ago

assuming I was the inspiration for the fix :D