PHP-Parser icon indicating copy to clipboard operation
PHP-Parser copied to clipboard

line separator problem in comments

Open zhaoyanliang2 opened this issue 5 years ago • 2 comments

https://github.com/nikic/PHP-Parser/blob/57b8673ea7373100e80aecdf18fe82c0dddbea28/lib/PhpParser/PrettyPrinterAbstract.php#L452

https://github.com/nikic/PHP-Parser/blob/57b8673ea7373100e80aecdf18fe82c0dddbea28/lib/PhpParser/Comment.php#L86

<?php
//Please use the Mac-style line-separator to test
if ( 1 ) {
	/**
	 * test
	 * test
	 */
	class test
	{

	}
}

If this file uses Mac-style line separator "\r", the comment will not be prettied very well.


The PhpParser assume that all files use Unix-style line separator "\n". This makes it difficult for us to change Any-style to Windows-style "\r\n" ( or Mac-style "\r") just by overriding resetState(), indent(), outdent().

class WindowsStylePrinter extends PhpParser\PrettyPrinter\Standard
{
	protected function resetState()
	{
		parent::resetState();
		$this->nl = "\r\n";
	}

	protected function indent()
	{
		$this->indentLevel += 4;
		$this->nl .= "\t";
	}

	protected function outdent()
	{
		assert($this->indentLevel >= 4);
		$this->indentLevel -= 4;
		$this->nl = substr($this->nl, 0, -1);
	}

	protected function pComments(array $comments) : string
	{
		//too difficult to do
	}
}

zhaoyanliang2 avatar Apr 12 '19 07:04 zhaoyanliang2

If this file uses Mac-style line separator "\r", the comment will not be prettied very well.

Is this a situation you encountered in real code, or just an academic concern? I believe Mac-style newlines haven't been in use anymore for something like two decades.

Better support for Windows newlines would be worthwhile, of course.

nikic avatar Apr 13 '19 19:04 nikic

I use mac-style as an example just to test it more easily without overriding any methods. The situation I actually encountered was formatting Unknown-style into Windows-style. So I wrote the WindowsStylePrinter class and found that "\r\n" in comments was replaced with "\r\r\n".

zhaoyanliang2 avatar Apr 13 '19 20:04 zhaoyanliang2

I've chosen to normalize CRLF to LF in getReformattedText() in https://github.com/nikic/PHP-Parser/commit/ad8daa12b2de5f4aed2a7eb821d2744b22f75e46, which should effectively fix this issue (for CRLF, I don't care about CR newlines).

nikic avatar May 21 '23 15:05 nikic