Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiline field markdowntable #289

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

BloggerBust-bot
Copy link

Depends on: #285

By replacing line-endings found within a field value with an HTML break element, the field value will render with multiple lines in a single table cell, rather than being broken up into multiple table rows.

The only difference between this PR and #285 is in Staticman.js on lines 280 to 282

Now the commit message will not break up multi-line field values into
multiple fields without values.
To handle both CR\LF and LF line endings I have chained two global
replace calls.
@VincentTam
Copy link
Contributor

Thanks for PR, even though I don't think this will be merged due to its dependence on #285. Actually, the part that actually does the job is

  Object.keys(fields).forEach(field => {
	    let fieldValue = fields[field]
	    fieldValue = typeof fieldValue === 'string' ? fieldValue.replace(/\r\n/g, '<br>').replace(/\n/g, '<br>') : fieldValue
	    table.push([field, fieldValue])
	  })

https://github.com/eduardoboucas/staticman/pull/289/files#diff-0eb30e85bf25fbc952a68845f6e23860

It is possible that a Staticman v3 user implement this part only without change the rest of the code?

VincentTam added a commit to VincentTam/staticman that referenced this pull request Jul 2, 2019
Implements first three commits of eduardoboucas#289
@VincentTam
Copy link
Contributor

VincentTam commented Jul 2, 2019

Update: I've tested the above code block without the rollback. It works light and right. Bravo ! Here's an example on GitLab.

@BloggerBust-bot
Copy link
Author

Update: I've tested the above code block without the rollback. It works light and right. Bravo ! Here's an example on GitLab.

I am glad to hear that you were able to get the fix working :-)

@BloggerBust-bot
Copy link
Author

Thanks for PR, even though I don't think this will be merged due to its dependence on #285.

Should I close PR #285? It doesn't seem that it is needed. When I created it, the master branch was broken and that was my way of getting things working again.

It is possible that a Staticman v3 user implement this part only without change the rest of the code?

It looks like you were able to get it working with v3. That is fabulous!

@VincentTam
Copy link
Contributor

VincentTam commented Jul 3, 2019

Should I close PR #285? It doesn't seem that it is needed. When I created it, the master branch was broken and that was my way of getting things working again.

I can't give a yes/no answer since I haven't tested that. Based on my testing, it seems that #231 doesn't work well with 4eb5ce2, which is included in your PR. I've chosen #231 since that's a continuation of Staticman's GitLab support, which is in fact my main goal of setting up another API instance, as the name "Staticman Lab" suggests. However, it doesn't hurt leaving it there, judging from the feedback from other users in #283.

It looks like you were able to get it working with v3. That is fabulous!

My instances are using the v3 URL scheme, but I'm not merging recent commits on the branches dev and master due to reasons described in #299 (comment). In short, I'm not a great fan of GitHub Apps, and @staticmanlab has to serve site owners affected by #227. Using GitHub Apps would break their "invitation". Moreover, a GitHub App also has its API limit. Finally, the official instances doesn't include support for personal Akismet API #195, so the spamming problem #175 remains unresolved in v3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants