Bug 123307

Summary: Find previous does nothing sometimes
Product: [Applications] kate Reporter: Brian DeRocher <brian.derocher>
Component: partAssignee: KWrite Developers <kwrite-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: mkaufmann
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Debian testing   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: sample text to distinguish working and broken cases
Possible patch

Description Brian DeRocher 2006-03-08 22:54:12 UTC 
Version:           4.5.1 (using KDE KDE 3.5.1)
Installed from:    Debian testing/unstable Packages

When searching for text in a document, i'll press control-f enter my text and press enter to close the dialog.  Then i press F3 multiple times as i browse down the document.  Then i press Shift-F3 to go backwards.  Sometimes i don't get back up to the first instance.  This bug appears to be similar to bug 95935.

I've produced an example.  The two search terms are where_price_2 and where_price_3.  2 is broken, but 3 works.  To demonstrate this it's important not to reach the bottom of the document.

	where_price_2 := 'broken search term';
	where_price_3 := 'working search term';
	if (price_gtbl_id = 271) or (price_gtbl_id = 272)  then
		-- next line is *a* cause of the problem
		where_price_2
		:= ' or a.' || where_price_2 || ' < b.' || where_price_2
		|| ' or a.' || where_price_3 || ' < b.' || where_price_3 
		-- going backwards stops two lines up
		-- but going backwards continues two lines up!
	-- now go backwards (shift-F3)
	where_price_2 this text must be here to search backwards
	where_price_3 this text must be here to search backwards

Also that 1 tab indent is important.  If the last where_price_2 is not indented, Find Previous just stays there on the penultimate line.  Just in case my text gets munged as i enter this bug, there are 12 lines above.
Comment 1 Brian DeRocher 2006-03-08 23:06:28 UTC 
Created attachment 15018 [details]
sample text to distinguish working and broken cases
Comment 2 Andreas Kling 2006-07-06 00:21:27 UTC 
Created attachment 16905 [details]
Possible patch

This patch fixes the problem by doing two things:
* When searching backwards, and there is a selection, start search at the start
of the selection instead of at the cursor (because of how the previous match is
highlighted.)
* Start one character before the starting point, to avoid hitting the previous
match again.

I do admit, this doesn't feel very clean. Some input would be appreciated.
Comment 3 Andreas Kling 2006-07-07 16:12:25 UTC 
SVN commit 559493 by kling:

Made backward searches work a lot better.

BUG: 123307
BUG: 130252


 M  +26 -4     katesearch.cpp  
 M  +1 -1      katesearch.h  


--- branches/KDE/3.5/kdelibs/kate/part/katesearch.cpp #559492:559493
@@ -141,7 +141,7 @@
     s.selEnd   = KateTextCursor( m_view->selEndLine(),   m_view->selEndCol()   );
     s.cursor   = s.flags.backward ? s.selEnd : s.selBegin;
   } else {
-    s.cursor = getCursor();
+    s.cursor = getCursor( searchFlags );
   }
 
   s.wrappedEnd = s.cursor;
@@ -207,7 +207,7 @@
     s.selEnd   = KateTextCursor( m_view->selEndLine(), m_view->selEndCol()   );
     s.cursor   = s.flags.backward ? s.selEnd : s.selBegin;
   } else {
-    s.cursor = getCursor();
+    s.cursor = getCursor( searchFlags );
   }
 
   s.wrappedEnd = s.cursor;
@@ -236,8 +236,8 @@
 
   searchFlags.fromBeginning = false;
   searchFlags.prompt = true; // ### why is the above assignment there?
-  s.cursor = getCursor();
 
+  s.cursor = getCursor( searchFlags );
   search( searchFlags );
 }
 
@@ -547,8 +547,15 @@
   return str;
 }
 
-KateTextCursor KateSearch::getCursor()
+KateTextCursor KateSearch::getCursor( SearchFlags flags )
 {
+  if (flags.backward && !flags.selected && view()->hasSelection())
+  {
+    // We're heading backwards (and not within a selection),
+    // the selection might start before the cursor.
+    return KMIN( KateTextCursor(view()->selStartLine(), view()->selStartCol()),
+                 KateTextCursor(view()->cursorLine(), view()->cursorColumnReal()));
+  }
   return KateTextCursor(view()->cursorLine(), view()->cursorColumnReal());
 }
 
@@ -586,6 +593,21 @@
   //kdDebug() << "Searching at " << line << ", " << col << endl;
 //   kdDebug()<<"KateSearch::doSearch: "<<line<<", "<<col<<", "<<backward<<endl;
 
+  if (backward)
+  {
+    KateDocCursor docCursor(line, col, doc());
+
+    // If we're at the top of the document, we're not gonna find anything, so bail.
+    if (docCursor.line() == 0 && docCursor.col() == 0)
+      return false;
+
+    // Move one step backward before searching, if this is a "find again", we don't
+    // want to find the same match.
+    docCursor.moveBackward(1);
+    line = docCursor.line();
+    col = docCursor.col();
+  }
+
   do {
       if( regExp ) {
         m_re = QRegExp( text, caseSensitive );
--- branches/KDE/3.5/kdelibs/kate/part/katesearch.h #559492:559493
@@ -139,7 +139,7 @@
     void skipOne();
 
     QString getSearchText();
-    KateTextCursor getCursor();
+    KateTextCursor getCursor( SearchFlags flags );
     bool doSearch( const QString& text );
     void exposeFound( KateTextCursor &cursor, int slen );
Comment 4 Andreas Kling 2006-07-09 23:07:09 UTC 
*** Bug 119924 has been marked as a duplicate of this bug. ***