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

Fix bug that \r\n are visible at the end of lines. #1

Closed
wants to merge 1 commit into from
Closed

Fix bug that \r\n are visible at the end of lines. #1

wants to merge 1 commit into from

Conversation

henry0312
Copy link

No description provided.

@henry0312 henry0312 closed this Jun 15, 2012
@henry0312 henry0312 reopened this Jun 15, 2012
@aramk aramk closed this Jun 16, 2012
@aramk
Copy link
Owner

aramk commented Jun 16, 2012

I've fixed this in 1.9.9, it was caused by using single quotes. That line is to ensure that any line using \r or \n uses \r\n so that it's windows compatible as well.

@henry0312
Copy link
Author

NO, NO!
Your change, "\r\n" is wrong.
This causes unnecessary empty lines on both Windows and Mac OS X, I confirmed.

@aramk
Copy link
Owner

aramk commented Jun 16, 2012

The regex "(\r(?!\n))|((?<!\r)\n)" doesn't create line breaks or carriage returns, it merely converts \r and \n to \r\n, but only if they aren't \r\n. If there's still a problem could you give an example?

@henry0312
Copy link
Author

Test – Crayon Syntax Highlighter | Re: no subject
http://henry.animeo.jp/wp/?p=1701

Please see.
line 2 and 8 are unnecessary empty lines.

I used the original code, http://privatepaste.com/056b54d69c

by the way, I use WordPress 3.4.

@aramk
Copy link
Owner

aramk commented Jun 16, 2012

Sorry about that, fixed in the last commit.

@henry0312
Copy link
Author

OK, the latest commit seems fine ;)

@aramk
Copy link
Owner

aramk commented Jun 16, 2012

Great! The problem was I hadn't accounted the \r\n into the code that split lines (It used $, which comes after \r and before \n)

@henry0312
Copy link
Author

This bug seems to appear again after commit 82d11a5

Test – Crayon Syntax Highlighter
http://henry.animeo.jp/wp/?p=1716

Please see.
line 2 and 8 are unnecessary empty lines.

I used the original code, http://privatepaste.com/056b54d69c

@aramk
Copy link
Owner

aramk commented Jun 24, 2012

Have a look at the latest commits. I'm going to work with git from now on rather than svn and only use it for releases. I'm making a new branch for the tag editor.

@henry0312
Copy link
Author

I pulled dba1714, but this problem isn't solved.

@aramk
Copy link
Owner

aramk commented Jun 24, 2012

Sorry, I discovered the cause after I wrote that. Try 071c38b.

@henry0312
Copy link
Author

Fixed this problem with 071c38b :D

@aramk
Copy link
Owner

aramk commented Jun 24, 2012

Great, in the future I'll make sure the repo is handled better now that I'm using Git primarily.

@henry0312
Copy link
Author

All right.
I will use github code , not archives on http://wordpress.org/extend/plugins/crayon-syntax-highlighter/

@aramk
Copy link
Owner

aramk commented Jun 24, 2012

Only if something is broken. Consider the tags on svn official and everything on github as development.

hinaloe added a commit to hinaloe/crayon-syntax-highlighter that referenced this pull request Feb 19, 2024
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.

2 participants