search

sintaxi / harp

Static Web Server/Generator/Bundler
http://harpjs.com
4.99k stars 346 forks source link

markdown table ignores empty cells #638

Closed capitalaslash closed 5 years ago

capitalaslash commented 5 years ago

If I try to serve the following markdown file I get a pipe in the second column instead of an empty cell

a | b | c
- | - | -
1 |   | 2
misterhtmlcss commented 5 years ago

I've attached what the code you provided does in an alternative editor. Is my version what you wanted? screen shot 2018-10-03 at 5 26 37 pm

misterhtmlcss commented 5 years ago

Please confirm what the issue is, so that I can categorize it and determine how we can assist you. Thank you for raising your issue!

capitalaslash commented 5 years ago

yes, the picture you posted is the expected behavior. let me know of any further detail you may need.

misterhtmlcss commented 5 years ago

@capitalaslash great.

Also if you can tell me about your OS, Browser, Harp CLI, what html pre-compiler you are using, version and anything else you think might be relevant as that's just the basics.

capitalaslash commented 5 years ago

OS: CentOS browser: chromium version: 0.28.1

I don't know what you mean with html pre-compiler? I am serving a super simple markdown website with a bunch a pages for local-use only with

NODE_ENV=production harp server <path> --port <port>
misterhtmlcss commented 5 years ago

@capitalaslash good to know. We are on version 0.29.x now. Can you update and confirm back to me the issue still exists?

What I was referring to was .ejs, .pug or .jade file types. These are HTML (template) based languages that compile into regular HTML dynamically.

I'll look into your issue tonight. Thank you for the update! Have a great day.

capitalaslash commented 5 years ago

oh yes, I'm using https://github.com/techstreams/harp-github-markdown to get a github style formatting, but I tested without it to check if the error was coming from that.

capitalaslash commented 5 years ago

confirmed in 0.29.0

misterhtmlcss commented 5 years ago

Too bad. Was hoping for a simple fix. This is getting very clear to me and I'll definitely have a good look tonight. Thank you for jumping through those hoops.

misterhtmlcss commented 5 years ago

@capitalaslash I can't see how the issue is bubbling up. I see it as a non-standard .md using tables. Broadly it's not a supported feature. I'll have to speak with one of the team leads to see if it was our intention to support it. I'll let you know.

misterhtmlcss commented 5 years ago

@sintaxi thoughts on this? Did we have an intention to support tables with .md?

sintaxi commented 5 years ago

@misterhtmlcss That could be as simple as changing the configuration in terraform or switching to a different markdown processor. If we are confident marked is the best processor perhaps we can make the config part of the harp public API. It would make for an easy win.

https://github.com/sintaxi/terraform/blob/master/lib/template/processors/md.js#L4

here are the marked configuration options...

https://marked.js.org/#/USING_ADVANCED.md#options

capitalaslash commented 5 years ago

@misterhtmlcss I understand that tables are not "standard" markdown (if you refer to the original implementation), but the table rendering is supported by your back-end, just in a buggy way. You should have it fixed (I hope!), or completely removed (to adhere to that "standard" markdown). I would like also to point out that there is a work-around for this bug, as you can use an unbreakable space in the column to obtain the expected behavior. Definitely not elegant or intuitive.

misterhtmlcss commented 5 years ago

@capitalaslash a PR was made by myself for this issue. @sintaxi is integrating some other bits and pieces as well and then we'll get it up on NPM for you to use. Talk soon!

misterhtmlcss commented 5 years ago

@sintaxi to me it looks like the 'marked'-down people are still struggling with issues related to scenarios like this and while I think if we upgrade our build to marked 0.5.x we may overcome this particular issue.

capitalaslash commented 5 years ago

I can confirm that marked 0.5 fixes this problem with markedjs/marked#1262

misterhtmlcss commented 5 years ago

We'll consider upgrading the Markedjs version to .5.x for inclusion into our 1.x launch. Thank you for confirming my understanding. Always nice to have two heads agree over one.

sintaxi commented 5 years ago

Nice work. I just upgraded to v0.5.1 and published terraform@1.8.0.