search

ckruse / cforum

https://github.com/ckruse/cforum_ex/
GNU Affero General Public License v3.0
22 stars 5 forks source link

Removed newlines to fix issue 641 #683

Closed CountOrlok closed 7 years ago

ckruse commented 7 years ago

I'm sorry, but this fix isn't sufficient.

Wie added the newlines to avoid the most common error with markdown: missing newlines between blocks. This drastically reduced code blocks not rendered as code blocks. If you remove them we have the same problem again.

I think what you actually have to do is look for the previous two characters and check if they're newlines. If yes, then don't add newlines. If no then add newlines. Do you agree?

CountOrlok commented 7 years ago

ACK. I did some additional testing and I think there is even more work required than you proposed, since inline code triggers a code block if followed by newline, which should not be the case either.

My code: doSomething()

Some plain text.

This is really a mess.

ckruse commented 7 years ago

Heh, this is always the case when the textarea contains newlines. Pretty bugged. I check the complete textarea for newlines instead of just the selected test:

            else if(content.indexOf('\n') > -1) {

content should've been selected.

CountOrlok commented 7 years ago

I'm going to change this. BTW, selected contains the return value of getSelection(), which is a plain object. The textual content is assigned to the variable chunk.

We should also consider to replace indexOf with includes to check for existance. The method is now widely supported and using it improves readability.

        else if (chunk.includes('\n')) {

Nevertheless, if we decide to do so, I'd recommend to add a polyfill for Internet Explorer and some older mobile browsers, until they're dead and buried. Your thoughts?

ckruse commented 7 years ago

Go for it. I trust your judgement.