Opened 10 years ago

Closed 7 years ago

Last modified 10 months ago

#157 closed Bugs (Accepted)

Tasks not reporting corrent line number in task view

Reported by: gushy Owned by: nobody
Priority: 5 Milestone:
Component: PHP Editor Version: None
Keywords: Cc:

Description (last modified by scorphus)

Tasks read from comments in the code do not show the
right line number (although clicking them takes you to
the correct line).


Change History (4)

comment:1 Changed 8 years ago by samage

  • Summary changed from Tasks not reporting right to Tasks not reporting corrent line number in task view
Logged In: YES 
user_id=1639026

I've confirmed this bug.
I've also created a small test page to use:

-----8<--------------
<?php

include_once("file.php");

function getDate()
{
	//This is the data to retrieve
	if (count($this->arrData)> 0)
	{
		//TODO: handle this condition
		print_r($this->$arrData);
	} 
	//TODO: what about an error condition?
}

function getPageFooter()
{
	// TODO: implement stub getPageFooter();
	die ("getPageFooter not implemented");	
}
//TODO: last item
//TODO: not quite
?>
-----8<--------8<-------

TODO Items are on lines 10, 13, 18, 21 and 22.

They are reported to be on 4, 6, 8, 10 and 11.

Confirmed with Eclipse 3.2.1 using phpEclipse 1.1.8 on
Ubuntu. (Sun jdk 1.5)

comment:2 Changed 7 years ago by scorphus

Logged In: YES 
user_id=987242
Originator: NO

This bug goes unnoticed when editing files with CRLF (\r\n) for newlines, which is the case of text files on Windows. When the newline is represented either with LF (Unix-like systems, Mac OS X and others) or CR (Mac OS up to 9) the Tasks view (and also the Problems view) doesn't show correct line numbers for some cases of files (with blank lines, or with lines of white spaces, and other cases you can experiment...).

The solution to this bug is quite simple although it was a bit hard to find.

The array (Scanner::lineEnds) that holds positions of every eol (end of line) in the text is incorrectly generated by Scanner::pushLineSeparator() - that fills the array - and Scanner::getNextToken() - where the bug resides: whenever there is a '\r' (CR) or '\n' (LF) the a new character gets read from the Scanner::source right after entering the first while loop, that causes the previously character read just before to be missed.

Suppose the occurrence of two newlines on the code. As of the case with '...\r\n\r\n...' (Windows) the first '\n' is missed, but the first newline was already counted and that's because line-numbering won't get messed up on the tasks view. But in case of two newlines on a Unix text file ('...\n\n...') the missed LF represents another newline.

It's a pretty silly bug :) and the solution is simple: read the new character from Scanner::source at the end of the while loop.

Here is the patch:

[patch]

Index: Scanner.java
===================================================================
RCS file: /cvsroot/phpeclipse/net.sourceforge.phpeclipse/src/net/sourceforge/phpdt/internal/compiler/parser/Scanner.java,v
retrieving revision 1.61
diff -u -r1.61 Scanner.java
--- Scanner.java	17 Mar 2007 13:54:41 -0000	1.61
+++ Scanner.java	22 Mar 2007 01:20:22 -0000
@@ -1438,8 +1438,6 @@
 
 					while ((currentCharacter == ' ')
 							|| Character.isWhitespace(currentCharacter)) {
-						startPosition = currentPosition;
-						currentCharacter = source[currentPosition++];
 						if ((currentCharacter == '\r')
 								|| (currentCharacter == '\n')) {
 							checkNonExternalizeString();
@@ -1449,6 +1447,8 @@
 								currentLine = null;
 							}
 						}
+						startPosition = currentPosition;
+						currentCharacter = source[currentPosition++];
 					}
 					if (tokenizeWhiteSpace
 							&& (whiteStart != currentPosition - 1)) {

[/patch]

Hope to solve more bugs!

Regards,
Pablo.

comment:3 Changed 7 years ago by dizmo

  • Status changed from assigned to closed

comment:4 Changed 10 months ago by scorphus

  • Description modified (diff)

Here is my previous comment with the patch, easier to read:

This bug goes unnoticed when editing files with CRLF (\r\n) for newlines, which is the case of text files on Windows. When the newline is represented either with LF (Unix-like systems, Mac OS X and others) or CR (Mac OS up to 9) the Tasks view (and also the Problems view) doesn't show correct line numbers for some cases of files (with blank lines, or with lines of white spaces, and other cases you can experiment...).

The solution to this bug is quite simple although it was a bit hard to find.

The array (Scanner::lineEnds) that holds positions of every eol (end of line) in the text is incorrectly generated by Scanner::pushLineSeparator() - that fills the array - and Scanner::getNextToken() - where the bug resides: whenever there is a '\r' (CR) or '\n' (LF) the a new character gets read from the Scanner::source right after entering the first while loop, that causes the previously character read just before to be missed.

Suppose the occurrence of two newlines on the code. As of the case with '...\r\n\r\n...' (Windows) the first '\n' is missed, but the first newline was already counted and that's because line-numbering won't get messed up on the tasks view. But in case of two newlines on a Unix text file ('...\n\n...') the missed LF represents another newline.

It's a pretty silly bug :) and the solution is simple: read the new character from Scanner::source at the end of the while loop.

Here is the patch:

Index: Scanner.java
===================================================================
RCS file: /cvsroot/phpeclipse/net.sourceforge.phpeclipse/src/net/sourceforge/phpdt/internal/compiler/parser/Scanner.java,v
retrieving revision 1.61
diff -u -r1.61 Scanner.java
--- Scanner.java	17 Mar 2007 13:54:41 -0000	1.61
+++ Scanner.java	22 Mar 2007 01:20:22 -0000
@@ -1438,8 +1438,6 @@

 					while ((currentCharacter == ' ')
 							|| Character.isWhitespace(currentCharacter)) {
-						startPosition = currentPosition;
-						currentCharacter = source[currentPosition++];
 						if ((currentCharacter == '\r')
 								|| (currentCharacter == '\n')) {
 							checkNonExternalizeString();
@@ -1449,6 +1447,8 @@
 								currentLine = null;
 							}
 						}
+						startPosition = currentPosition;
+						currentCharacter = source[currentPosition++];
 					}
 					if (tokenizeWhiteSpace
 							&& (whiteStart != currentPosition - 1)) {

Hope to solve more bugs!

Regards,
Pablo.

Note: See TracTickets for help on using tickets.