eduardoboucas / staticman

💪 User-generated content for Git-powered websites
https://staticman.net
MIT License
2.42k stars 542 forks source link

Fix line-break formatting in pull requests #90

Open klundberg opened 7 years ago

klundberg commented 7 years ago

I just got this PR from staticman: https://github.com/klundberg/klundberg.github.io/pull/4



~~~I apologize if this already exists and I don't have my site properly configured, but when I set things up I didn't see anything along these lines of only allowing specific form fields to be sent.~~~

My mistake, it's seems like there was no clever spammer behavior here that can be easily prevented. The problem here that confused me appears to be that the line breaks in the message mess up formatting of the PR, which made it look like there were extra form fields added to embed some targeted spam only intended for blog owners. The message in that PR should probably fit entirely inside one markdown table cell, if that's possible to do.

Thank you!
Stanko commented 7 years ago

I can confirm this.

But it can be solved easily, when generating a table: https://github.com/eduardoboucas/staticman/blob/ecf03862a0164e5d0b2575e2bf926a41bf034c84/lib/Staticman.js#L201-L203

Just remove new lines and replace them with breaks <br />.

  Object.keys(fields).forEach(field => {
    table.push([field, fields[field].replace(/\n/gm, '<br />')])
  })

Afaik this is the only way to add newlines to a markdown table.

~I will do a PR for this soon.~ EDIT: This is a one line fix, but I'm not sure how to test it before making a PR :/

Stanko commented 6 years ago

Bump, any news on this one?

JokerQyou commented 6 years ago

Just got two comments with the same issue, and each of them renders differently. One looks exactly same like the PR in @klundberg 's repo. And the other look like this:

image

Furthermore, I don't understand where the fields tags and layout came from. I've never configured them in allowedFields in staticman.yml.

BloggerBust-bot commented 5 years ago

@JokerQyou The code is only replacing LF, but I suspect your example had CR/LF. In PR #289 I chained two calls to replace to handle both scenarios.

VincentTam commented 5 years ago

@BloggerBust-bot Your PR contains an unnecessary dependence on #285, so I created another one for the convenience of v3 users. Anyways, great work!