From 654cded167a3d5ce3ee8ec171a698268ef2d589f Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Mon, 15 Jan 2018 16:14:21 +0100
Subject: [PATCH] Partial cleanup of the row selection code

This is preliminary work, this code still feels too complicated for
its own good.

Let Row::isMarginSelected return false when Row::selection() is false
(the other changes are indentation).

This allows to remove the test for selection() in
setSelectionAndMargins, so that begin/end_margin_sel are always set
correctly.

Add clearSelectionAndMargins() instead of calling directly setSelection
(which is now private) with arguments (-1, -1).

Fixes bug #10972.
---
 src/Row.cpp         | 52 +++++++++++++++++++++++++--------------------
 src/Row.h           | 18 +++++++++-------
 src/TextMetrics.cpp |  2 +-
 3 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/src/Row.cpp b/src/Row.cpp
index e105ae2333..697e526094 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -175,25 +175,24 @@ bool Row::isMarginSelected(bool left, DocIterator const & beg,
 	pos_type const sel_pos = left ? sel_beg : sel_end;
 	pos_type const margin_pos = left ? pos_ : end_;
 
-	// Is the chosen margin selected ?
-	if (sel_pos == margin_pos) {
-		if (beg.pos() == end.pos())
-			// This is a special case in which the space between after
-			// pos i-1 and before pos i is selected, i.e. the margins
-			// (see DocIterator::boundary_).
-			return beg.boundary() && !end.boundary();
-		else if (end.pos() == margin_pos)
-			// If the selection ends around the margin, it is only
-			// drawn if the cursor is after the margin.
-			return !end.boundary();
-		else if (beg.pos() == margin_pos)
-			// If the selection begins around the margin, it is
-			// only drawn if the cursor is before the margin.
-			return beg.boundary();
-		else
-			return true;
-	}
-	return false;
+	// Is there a selection and is the chosen margin selected ?
+	if (!selection() || sel_pos != margin_pos)
+		return false;
+	else if (beg.pos() == end.pos())
+		// This is a special case in which the space between after
+		// pos i-1 and before pos i is selected, i.e. the margins
+		// (see DocIterator::boundary_).
+		return beg.boundary() && !end.boundary();
+	else if (end.pos() == margin_pos)
+		// If the selection ends around the margin, it is only
+		// drawn if the cursor is after the margin.
+		return !end.boundary();
+	else if (beg.pos() == margin_pos)
+		// If the selection begins around the margin, it is
+		// only drawn if the cursor is before the margin.
+		return beg.boundary();
+	else
+		return true;
 }
 
 
@@ -202,10 +201,17 @@ void Row::setSelectionAndMargins(DocIterator const & beg,
 {
 	setSelection(beg.pos(), end.pos());
 
-	if (selection()) {
-		change(end_margin_sel, isMarginSelected(false, beg, end));
-		change(begin_margin_sel, isMarginSelected(true, beg, end));
-	}
+	change(end_margin_sel, isMarginSelected(false, beg, end));
+	change(begin_margin_sel, isMarginSelected(true, beg, end));
+}
+
+
+void Row::clearSelectionAndMargins() const
+{
+	change(sel_beg, -1);
+	change(sel_end, -1);
+	change(end_margin_sel, false);
+	change(begin_margin_sel, false);
 }
 
 
diff --git a/src/Row.h b/src/Row.h
index e45fa4e8c9..a42d91d3f1 100644
--- a/src/Row.h
+++ b/src/Row.h
@@ -163,18 +163,18 @@ public:
 	bool changed() const { return changed_; }
 	///
 	void changed(bool c) const { changed_ = c; }
-	/// Set the selection begin and end.
-	/**
-	  * This is const because we update the selection status only at draw()
-	  * time.
-	  */
-	void setSelection(pos_type sel_beg, pos_type sel_end) const;
 	///
 	bool selection() const;
-	/// Set the selection begin and end and whether the left and/or right
-	/// margins are selected.
+	/**
+	 * Set the selection begin and end and whether the left and/or
+	 * right margins are selected.
+	 * This is const because we update the selection status only at
+	 * draw() time.
+	 */
 	void setSelectionAndMargins(DocIterator const & beg,
 		DocIterator const & end) const;
+	/// no selection on this row.
+	void clearSelectionAndMargins() const;
 
 	///
 	void pit(pit_type p) { pit_ = p; }
@@ -323,6 +323,8 @@ private:
 	  */
 	bool isMarginSelected(bool left, DocIterator const & beg,
 		DocIterator const & end) const;
+	/// Set the selection begin and end.
+	void setSelection(pos_type sel_beg, pos_type sel_end) const;
 
 	/**
 	 * Returns true if a char or string with font \c f and change
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 3b4e1ff5d0..fedb773085 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1878,7 +1878,7 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type const pit, int const
 		if (selection)
 			row.setSelectionAndMargins(sel_beg_par, sel_end_par);
 		else
-			row.setSelection(-1, -1);
+			row.clearSelectionAndMargins();
 
 		// The row knows nothing about the paragraph, so we have to check
 		// whether this row is the first or last and update the margins.
-- 
2.39.5