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.

diff --git a/src/Row.cpp b/src/Row.cpp
index e105ae2333576d80dc84e59ea1e17b8a0e831bb6..697e526094b7f399801f4d62d24f7b39f5fc503a 100644 (file)
--- 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 e45fa4e8c99e4f8d5857a95c9e68a830847b0917..a42d91d3f18d30e69c38f56edcd6bdda62770fc5 100644 (file)
--- 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 3b4e1ff5d0e41f23e42eaffd7bf1c0ef6a58359f..fedb7730850aaf6693178536ffafc7ef51d3745e 100644 (file)
--- 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.