Bug 123307 - Find previous does nothing sometimes
Summary: Find previous does nothing sometimes
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: part (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR normal with 30 votes (vote)
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
: 119924 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-03-08 22:54 UTC by Brian DeRocher
Modified: 2006-07-09 23:07 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
sample text to distinguish working and broken cases (569 bytes, text/plain)
2006-03-08 23:06 UTC, Brian DeRocher
Details
Possible patch (1.22 KB, patch)
2006-07-06 00:21 UTC, Andreas Kling
Details

Note You need to log in before you can comment on or make changes to this bug.
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. ***