On Fri, Jan 22, 2016 at 06:36:03PM -0500, Guillaume Munch wrote:
> Le 21/01/2016 05:19, Enrico Forestieri a écrit :
> >
> >Please, find attached an updated patch that moves the check for
> >separators on mouse clicks to a centralized place (also accounting
> >for shift-clicks and other modifiers).
> 
> Thank you for the patch, I started testing it but I would be more
> comfortable in testing it in the long term. I do not know enough about the
> architecture of mouse events to be personally confident about it.

It should be pretty safe in this regard.

> For this patch and for the one at Wed, Jan 20, 2016 at 02:18, I also noticed
> that it skips the position right before the separator: it jumps from the
> beginning of the last word to the beginning of the first paragraph. It would
> be desirable to stop after the last word

This is the same behavior you get with the newline inset, and it was
indeed modeled upon that. However, you are right that it should be
different, maybe. The attached patch follows your suggestion.

> For this patch I noticed that clicking in the right margin after a separator
> jumps to an unexpected location if the separator is in a collapsible inset
> (either at the start or the end of the inset).

This was a thinko of mine. I was trying to unnecessarily do more than
strictly needed. Should be corrected in the attached patch.

> Maybe we can go with the improvements you already made for beta, and commit
> this particular patch to master after release.

Fair enough. Please find attached the next iteration of the patch in
case you want to evaluate it. Note that I could not solve the problem
that a separator may be randomly selected after multiple clicks.
However, I think that this is a minor annoyance.

-- 
Enrico
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 81ebe79..9d5f538 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -2203,9 +2203,13 @@ void BufferView::mouseEventDispatch(FuncRequest const & 
cmd0)
                // inset (depending on cmd.x(), cmd.y()). This is needed for
                // editing to fix bug 9628, but e.g. the context menu needs a
                // cursor in front of the inset.
-               if (inset->hasSettings() &&
+               if ((inset->hasSettings() || inset->lyxCode() == 
SEPARATOR_CODE) &&
                    cur.nextInset() != inset && cur.prevInset() == inset)
                        cur.posBackward();
+       } else if (cur.inTexted() && cur.pos()
+                       && cur.paragraph().isEnvSeparator(cur.pos() - 1)) {
+               // Always place cursor in front of a separator inset.
+               cur.posBackward();
        }
 
        // Put anchor at the same position.
@@ -2512,12 +2516,8 @@ bool BufferView::mouseSetCursor(Cursor & cur, bool 
select)
        // FIXME: (2) if we had a working InsetText::notifyCursorLeaves,
        // the leftinset bool would not be necessary (badcursor instead).
        bool update = leftinset;
-       if (!do_selection && d->cursor_.inTexted()) {
+       if (!do_selection && d->cursor_.inTexted())
                update |= checkDepm(cur, d->cursor_);
-               if (cur.inTexted() && cur.pos()
-                       && cur.paragraph().isEnvSeparator(cur.pos() - 1))
-                   cur.posBackward();
-       }
 
        if (!do_selection)
                d->cursor_.resetAnchor();
diff --git a/src/Text.cpp b/src/Text.cpp
index 187e387..847fe88 100644
--- a/src/Text.cpp
+++ b/src/Text.cpp
@@ -1110,7 +1110,7 @@ bool Text::cursorForwardOneWord(Cursor & cur)
        Paragraph const & par = cur.paragraph();
 
        // Paragraph boundary is a word boundary
-       if (pos == lastpos) {
+       if (pos == lastpos || (pos + 1 == lastpos && par.isEnvSeparator(pos))) {
                if (pit != cur.lastpit())
                        return setCursor(cur, pit + 1, 0);
                else
@@ -1143,6 +1143,10 @@ bool Text::cursorForwardOneWord(Cursor & cur)
                             ++pos;
        }
 
+       // Don't skip a separator inset at the end of a paragraph
+       if (pos == lastpos && pos && par.isEnvSeparator(pos - 1))
+               --pos;
+
        return setCursor(cur, pit, pos);
 }
 
@@ -1156,8 +1160,14 @@ bool Text::cursorBackwardOneWord(Cursor & cur)
        Paragraph & par = cur.paragraph();
 
        // Paragraph boundary is a word boundary
-       if (pos == 0 && pit != 0)
-               return setCursor(cur, pit - 1, getPar(pit - 1).size());
+       if (pos == 0 && pit != 0) {
+               Paragraph & prevpar = getPar(pit - 1);
+               pos = prevpar.size();
+               // Don't stop after an environment separator
+               if (pos && prevpar.isEnvSeparator(pos - 1))
+                       --pos;
+               return setCursor(cur, pit - 1, pos);
+       }
 
        if (lyxrc.mac_like_cursor_movement) {
                // Skip through punctuation and spaces.
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 325fb6c..9cd03b4 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1581,7 +1581,8 @@ bool TextMetrics::cursorEnd(Cursor & cur)
                        boundary = true;
                else
                        --end;
-       }
+       } else if (cur.paragraph().isEnvSeparator(end-1))
+               --end;
        return text_->setCursor(cur, cur.pit(), end, true, boundary);
 }
 

Reply via email to