Tables #66

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

redsun82 commented Oct 31, 2012

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

Collaborator

ashb commented Oct 31, 2012

Thanks for this.

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

Contributor

redsun82 commented Oct 31, 2012

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.

Contributor

redsun82 commented Oct 31, 2012

Here they are.

Why hasn't this been merged yet?

Collaborator

ashb commented Jul 26, 2013

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.

ashb closed this in 984a0ec Jul 26, 2013

Great, will test as soon as you release this

Collaborator

ashb commented Jul 26, 2013

@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

Collaborator

ashb commented Jul 26, 2013

@vanthome v0.5.0 just published to NPM.

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

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

Collaborator

ashb commented Jul 26, 2013

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

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.

Collaborator

ashb commented Jul 26, 2013

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

no error, just the tables are rendered plain

Collaborator

ashb commented Jul 26, 2013

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.

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

Owner

evilstreak commented Jul 26, 2013

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

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"]
Owner

evilstreak commented Jul 26, 2013

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.

This works, excellent, thx!

saelfaer referenced this pull request Oct 16, 2013

Open

are tables supported #141

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment