[LyX/2.5.x] Improve simple search within selection

Juergen Spitzmueller <[email protected]> Mon, 18 May 2026 17:46:09 +0000
Newsgroups gmane.editors.lyx.cvs
Message-ID <[email protected]>
commit c60ec0a3009955b049d9303855db59e6cb0a2327
Author: Juergen Spitzmueller <[email protected]>
Date:   Sat Mar 28 12:44:10 2026 +0100

    Improve simple search within selection
    
    When searching/replacing within a selection, remember the selection range
    beyond the first match.
    
    To this end, we record whether a selection has been made in result of
    a search match and differentiate this kind of selection from others
    (those within which we ought to search).
    
    In result, LyX (simple) search/replace within selection does no longer
    work in the restricted way (like LibreOffice) to only provide one hit
    within a selection, it keeps on searching for more hits until the
    selection end is reached and then asks whether it should continue
    searching outside (like M$ Word).
    It also does not moan about selection end being reached when it made the
    selection itself (to highlight a match).
    
    (cherry picked from commit 643e2bb0f9207f52ca6bee6257ffd0221a089c73)
---
 src/BufferView.cpp |  5 +++-
 src/BufferView.h   |  3 +-
 src/Cursor.cpp     |  9 ++++--
 src/Cursor.h       |  6 ++++
 src/lyxfind.cpp    | 80 +++++++++++++++++++++++++++++++++++-------------------
 src/lyxfind.h      |  2 +-
 status.25x         |  3 ++
 7 files changed, 74 insertions(+), 34 deletions(-)

diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index f6c672fe02..48bcea3fc3 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -3235,7 +3235,8 @@ bool BufferView::mouseSetCursor(Cursor & cur, bool const select)
 
 
 void BufferView::putSelectionAt(DocIterator const & cur,
-				int length, bool backwards)
+				int length, bool backwards,
+				bool const searchhit)
 {
 	d->cursor_.clearSelection();
 
@@ -3248,6 +3249,8 @@ void BufferView::putSelectionAt(DocIterator const & cur,
 		} else
 			d->cursor_.setSelection(d->cursor_, length);
 	}
+	if (searchhit)
+		d->cursor_.setSearchMatchSelection(true);
 }
 
 
diff --git a/src/BufferView.h b/src/BufferView.h
index eca7f84ccc..50f783f37e 100644
--- a/src/BufferView.h
+++ b/src/BufferView.h
@@ -306,7 +306,8 @@ public:
 	 * cur + \c length.
 	 */
 	void putSelectionAt(DocIterator const & cur,
-		int length, bool backwards);
+		int length, bool backwards,
+		bool const searchhit = false);
 	/// set a selection between \p from and \p to
 	void setSelection(DocIterator const & from,
 			 DocIterator const & to);
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index cc06268483..52c8bf2821 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -123,19 +123,19 @@ DocIterator bruteFind(Cursor const & c, int x, int y)
 
 CursorData::CursorData()
 	: DocIterator(), anchor_(), selection_(false), mark_(false),
-	  word_selection_(false), current_font(inherit_font)
+	  word_selection_(false), search_match_selection_(false), current_font(inherit_font)
 {}
 
 
 CursorData::CursorData(Buffer * buffer)
 	: DocIterator(buffer), anchor_(), selection_(false), mark_(false),
-	  word_selection_(false), current_font(inherit_font)
+	  word_selection_(false), search_match_selection_(false), current_font(inherit_font)
 {}
 
 
 CursorData::CursorData(DocIterator const & dit)
 	: DocIterator(dit), anchor_(), selection_(false), mark_(false),
-	  word_selection_(false), current_font(inherit_font)
+	  word_selection_(false), search_match_selection_(false), current_font(inherit_font)
 {}
 
 
@@ -251,6 +251,7 @@ CursorSlice CursorData::normalAnchor() const
 void CursorData::setSelection()
 {
 	selection(true);
+	setSearchMatchSelection(false);
 	if (idx() == normalAnchor().idx() &&
 	    pit() == normalAnchor().pit() &&
 	    pos() == normalAnchor().pos())
@@ -262,6 +263,7 @@ void CursorData::setSelection(DocIterator const & where, int n)
 {
 	setCursor(where);
 	selection(true);
+	setSearchMatchSelection(false);
 	anchor_ = where;
 	pos() += n;
 }
@@ -500,6 +502,7 @@ void CursorData::clearSelection()
 {
 	selection(false);
 	setWordSelection(false);
+	setSearchMatchSelection(false);
 	setMark(false);
 	resetAnchor();
 }
diff --git a/src/Cursor.h b/src/Cursor.h
index c2fcf63135..879dce1033 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -123,6 +123,10 @@ public:
 	bool mark() const { return mark_; }
 	/// did we place the anchor?
 	void setMark(bool mark) { mark_ = mark; }
+	/// Set search match selection mode
+	void setSearchMatchSelection(bool set) { search_match_selection_ = set; }
+	/// Are we in search match selection mode?
+	bool searchMatchSelection() const { return search_match_selection_; }
 	///
 	void setSelection();
 	/// set selection at given position
@@ -229,6 +233,8 @@ private:
 	bool mark_;
 	/// are we in word-selection mode? This is set when double clicking.
 	bool word_selection_;
+	/// are we in search match selection mode?
+	bool search_match_selection_;
 
 	/// the start of the new born word
 	DocIterator new_word_;
diff --git a/src/lyxfind.cpp b/src/lyxfind.cpp
index 46180ceea4..62a8fa15fe 100644
--- a/src/lyxfind.cpp
+++ b/src/lyxfind.cpp
@@ -239,8 +239,9 @@ int findForward(DocIterator & cur, DocIterator const & endcur,
 		bool find_del = true, bool onlysel = false)
 {
 	for (; cur; cur.forwardChar()) {
-		if (onlysel && cur.inTexted() && endcur.pit() == cur.pit()
-		    && endcur.idx() == cur.idx() && endcur.pos() < cur.pos())
+		if (onlysel && cur.inTexted()
+		    && (endcur.pit() < cur.pit() || (endcur.pit() == cur.pit()
+		    && endcur.idx() == cur.idx() && endcur.pos() < cur.pos())))
 			break;
 		if (cur.inTexted()) {
 			int len = match(cur.paragraph(), cur.pos(), find_del);
@@ -258,8 +259,9 @@ int findBackwards(DocIterator & cur, DocIterator const & endcur,
 {
 	while (cur) {
 		cur.backwardChar();
-		if (onlysel && cur.inTexted() && endcur.pit() == cur.pit()
-		    && endcur.idx() == cur.idx() && endcur.pos() > cur.pos())
+		if (onlysel && cur.inTexted()
+		    && (endcur.pit() > cur.pit() || (endcur.pit() == cur.pit()
+		    && endcur.idx() == cur.idx() && endcur.pos() > cur.pos())))
 			break;
 		if (cur.inTexted()) {
 			int len = match(cur.paragraph(), cur.pos(), find_del);
@@ -283,10 +285,13 @@ bool searchAllowed(docstring const & str)
 } // namespace
 
 
+DocIterator selection_search_start;
+DocIterator selection_search_end;
+
 bool findOne(BufferView * bv, docstring const & searchstr,
 	     bool case_sens, bool whole, bool forward,
 	     bool find_del, bool check_wrap, bool const auto_wrap,
-	     bool instant, bool onlysel)
+	     bool instant, bool onlysel, bool fromrep)
 {
 	bool const had_selection = bv->cursor().selection();
 
@@ -302,10 +307,14 @@ bool findOne(BufferView * bv, docstring const & searchstr,
 	if (!searchAllowed(searchstr))
 		return false;
 
-	DocIterator const startcur = bv->cursor().selectionBegin();
-	DocIterator const endcur = bv->cursor().selectionEnd();
+	if (!fromrep && (!had_selection || !bv->cursor().searchMatchSelection())) {
+		selection_search_start = bv->cursor().selectionBegin();
+		selection_search_end = bv->cursor().selectionEnd();
+	}
+
+	bool const in_new_selection = onlysel && had_selection && !bv->cursor().searchMatchSelection();
 
-	if (onlysel && had_selection) {
+	if (in_new_selection) {
 		docstring const matchstring = bv->cursor().selectionAsString(false);
 		docstring const lcmatchsting = support::lowercase(matchstring);
 		if (matchstring == searchstr || (!case_sens && lcmatchsting == lowercase(searchstr))) {
@@ -324,28 +333,35 @@ bool findOne(BufferView * bv, docstring const & searchstr,
 	}
 
 	DocIterator cur = forward
-		? ((instant || onlysel) ? bv->cursor().selectionBegin() : bv->cursor().selectionEnd())
-		: ((instant || onlysel) ? bv->cursor().selectionEnd() : bv->cursor().selectionBegin());
+		? ((instant || in_new_selection) ? bv->cursor().selectionBegin() : bv->cursor().selectionEnd())
+		: ((instant || in_new_selection) ? bv->cursor().selectionEnd() : bv->cursor().selectionBegin());
 
 	MatchString const match(searchstr, case_sens, whole);
 
+	// Set only_selection to false if we search outside a set selection range
+	onlysel &= selection_search_start != selection_search_end;
+
 	int match_len = forward
-		? findForward(cur, endcur, match, find_del, onlysel)
-		: findBackwards(cur, startcur, match, find_del, onlysel);
+		? findForward(cur, selection_search_end, match, find_del, onlysel)
+		: findBackwards(cur, selection_search_start, match, find_del, onlysel);
 
 	if (match_len > 0)
-		bv->putSelectionAt(cur, match_len, !forward);
-	else if (onlysel && had_selection) {
+		bv->putSelectionAt(cur, match_len, !forward, true);
+	else if (onlysel) {
 		docstring q = _("The search string was not found within the selection.\n"
 				"Continue search outside?");
 		int search_answer = frontend::Alert::prompt(_("Search outside selection?"),
 			q, 0, 1, _("&Yes"), _("&No"));
 		if (search_answer == 0) {
 			bv->clearSelection();
+			selection_search_start = bv->cursor().selectionBegin();
+			selection_search_end = bv->cursor().selectionEnd();
 			if (findOne(bv, searchstr, case_sens, whole, forward,
 				    find_del, check_wrap, auto_wrap, false, false))
 				return true;
-		}
+		} else
+			// restore original selection
+			bv->setSelection(selection_search_start, selection_search_end);
 		return false;
 	}
 	else if (check_wrap) {
@@ -381,12 +397,12 @@ bool findOne(BufferView * bv, docstring const & searchstr,
 				    find_del, false, false, false, false))
 				return true;
 		}
-		bv->setCursor(startcur);
+		bv->setCursor(selection_search_start);
 
 		// restore original selection
 		if (had_selection) {
 			bv->cursor().resetAnchor();
-			bv->setSelection(startcur, endcur);
+			bv->setSelection(selection_search_start, selection_search_end);
 		}
 		return false;
 	}
@@ -470,18 +486,18 @@ int replaceAll(BufferView * bv,
 }
 
 
-// the idea here is that we are going to replace the string that
+// The idea here is that we are going to replace the string that
 // is selected IF it is the search string.
-// if there is a selection, but it is not the search string, then
-// we basically ignore it. (FIXME We ought to replace only within
-// the selection.)
-// if there is no selection, then:
-//  (i) if some search string has been provided, then we find it.
+// If there is a selection, but it is not the search string, then
+// we search for a hit in this selection if "only selection" is set,
+// otherwise we search after the selection.
+// If there is no selection, then:
+//  (i) If some search string has been provided, then we find it.
 //      (think of how the dialog works when you hit "replace" the
 //      first time.)
-// (ii) if no search string has been provided, then we treat the
-//      word the cursor is in as the search string. (why? i have no
-//      idea.) but this only works in text?
+// (ii) If no search string has been provided, then we treat the
+//      word the cursor is in as the search string. ("Why? I have no
+//      idea.) But this only works in text?
 //
 // returns the number of replacements made (one, if any) and
 // whether anything at all was done.
@@ -491,7 +507,8 @@ pair<bool, int> replaceOne(BufferView * bv, docstring searchstr,
 			   bool onlysel)
 {
 	Cursor & cur = bv->cursor();
-	if (!cur.selection() || onlysel) {
+	bool const in_new_selection = onlysel && !bv->cursor().searchMatchSelection();
+	if (!cur.selection() || in_new_selection) {
 		// no selection, non-empty search string: find it
 		if (!searchstr.empty()) {
 			bool const found = findOne(bv, searchstr, case_sens, whole,
@@ -535,6 +552,13 @@ pair<bool, int> replaceOne(BufferView * bv, docstring searchstr,
 		return make_pair(false, 0);
 
 	cap::replaceSelectionWithString(cur, replacestr);
+	if (onlysel && selection_search_start != selection_search_end
+	    && selection_search_end.pit() == cur.pit()
+	    && selection_search_end.idx() == cur.idx()) {
+		selection_search_end.pos() += replacestr.length() - searchstr.length();
+		LASSERT(selection_search_end.pos() >= 0 && selection_search_end.pos() <= cur.lastpos(),
+			selection_search_end.pos() = 0);
+	}
 	if (forward) {
 		cur.pos() += replacestr.length();
 		LASSERT(cur.pos() <= cur.lastpos(),
@@ -542,7 +566,7 @@ pair<bool, int> replaceOne(BufferView * bv, docstring searchstr,
 	}
 	if (findnext)
 		findOne(bv, searchstr, case_sens, whole,
-			forward, false, findnext, wrap, false, onlysel);
+			forward, false, findnext, wrap, false, onlysel, true);
 
 	return make_pair(true, 1);
 }
diff --git a/src/lyxfind.h b/src/lyxfind.h
index 84d2400e2b..bb0c354ace 100644
--- a/src/lyxfind.h
+++ b/src/lyxfind.h
@@ -73,7 +73,7 @@ bool findOne(BufferView * bv, docstring const & searchstr,
 	     bool case_sens, bool whole, bool forward,
 	     bool find_del = true, bool check_wrap = false,
 	     bool const auto_wrap = false, bool instant = false,
-	     bool onlysel = false);
+	     bool onlysel = false, bool fromrep = false);
 
 /** Parse the string encoding of the replace request that is found in
  *  \c ev.argument and act on it.
diff --git a/status.25x b/status.25x
index 708f54886a..5d8ad627ae 100644
--- a/status.25x
+++ b/status.25x
@@ -35,6 +35,9 @@ What's new
 - It is now possible in the tabular creation dialog (Inset > Table) to
   insert a multi-page table (optionally with caption) directly.
 
+- Simple search within selection now remembers the initial selection
+  beyond the first match (bug 13293).
+
 
 * DOCUMENTATION AND LOCALIZATION
 
-- 
lyx-cvs mailing list
[email protected]
https://lists.lyx.org/mailman/listinfo/lyx-cvs