evilstreak / markdown-js

A Markdown parser for javascript
7.7k stars 863 forks source link

Tables #66

Closed redsun82 closed 11 years ago

redsun82 commented 11 years ago

Hi, I implemented simple tables for the maruku dialect (more or less following PHP Markdown Extra's implementation), as I needed them for a project of mine where I use this lib. Tests are still missing though...

A difference is that this implementation does not force cells in a row to be created up to the maximum width of the table... don't know yet if it's a big deal.

There are also some other minor changes that ended up in the code: the correction of a leaking global variable, the parameterisation of escaped characters per dialect (markdown extra apparently needs escaping for : and |), and a break statetement to exit a loop cheking for an empty dictionary.

edit: corrected typo

ashb commented 11 years ago

Thanks for this.

Could we have some tests that cover at least a few common cases of tables even if they are not exhaustive?

redsun82 commented 11 years ago

Sure, I'll try to have them up by today or tomorrow.

2012/10/31 Ash Berlin notifications@github.com

Thanks for this.

Could we have some tests that cover at least a few common cases of tables even if they are not exhaustive?

— Reply to this email directly or view it on GitHubhttps://github.com/evilstreak/markdown-js/pull/66#issuecomment-9946506.

redsun82 commented 11 years ago

Here they are.

vanthome commented 11 years ago

Why hasn't this been merged yet?

ashb commented 11 years ago

Two reasons, the main one being I'd forgotten about it - sorry. The second is that the change to how escapes are checked doesn't work (i.e. the tests don't pass) - I'm looking at this now.

vanthome commented 11 years ago

Great, will test as soon as you release this

ashb commented 11 years ago

@vanthome Just tidying up a few things and aiming to fix one more thing then a 0.5 will be on its way to npm

ashb commented 11 years ago

@vanthome v0.5.0 just published to NPM.

@redsun82 Thanks for this - sorry it took so long to merge.

vanthome commented 11 years ago

Hmm, are you sure that you pushed the latest version to GH?, I'm using this version and it does not seem to work :(

ashb commented 11 years ago

You'll need to make sure you select the Maruku dialect - the default mode of operation is compatible with Gruber's vanilla script.

vanthome commented 11 years ago

Ahh ok, I document this, but it still won't work:

I'm using it in the browser and doing this:

  markdown.Markdown('Maruku');
  html = markdown.toHTML(data);

And I took my table from your test data.

ashb commented 11 years ago

Do you get an error or it just doesn't produce the output you expected?

vanthome commented 11 years ago

no error, just the tables are rendered plain

ashb commented 11 years ago

Oh I see. markdown.Markdown('Maruku') creates an instance with the dialect set, but you've thrown that away.

html = markdown.toHTML(data, "Maruku");

should work for you.

vanthome commented 11 years ago

I tried this actually before but try to do this in the browser console of a page where markdown-js is included:

markdown.Markdown('Maruku');

you will see that it returns undefined

evilstreak commented 11 years ago

You need to use new markdown.Markdown('Maruku');

vanthome commented 11 years ago

I think this does not work in the browser. When I try this in console, I receive a useless object:

var test = new markdown.Markdown('Maruku');
Object.keys(test);
["dialect", "em_state", "strong_state", "debug_indent"]
evilstreak commented 11 years ago

Sorry, my comment was a bit hasty and not all that helpful.

The exposed parse function uses this approach. It's really only necessary if you want to muck around with the intermediate trees, rather than going from a Markdown string to an HTML string.

If you just want to go from string to string with a specific dialect, then as @ashb said you just want to pass the dialect into your toHTML call.

vanthome commented 11 years ago

This works, excellent, thx!