On Fri, Jun 20, 2014 at 10:40 AM, Jean-Marc Lasgouttes
<lasgout...@lyx.org> wrote:
> 18/06/2014 14:49, Scott Kostyshak:
> What about having Cursor::selection() return false if selection is empty?
Attached is my attempt to do this. I have no idea what this code does
though. Surprisingly the attempt seems to partially work (although I'm
sure it's still poorly written) if the selection is not started with
an inset. For example, if the cursor is just behind a note inset or a
table, selecting it will not work. If it is a math inset it crashes.
>> I don't know how to fix these at the core, in Cursor::selHandle(bool
>> sel) or Cursor::setSelection(). I think the following comment in
>> Cursor::setSelection is relevant:
>>
>> // A selection with no contents is not a selection
>> // FIXME: doesnt look ok
>> if (idx() == normalAnchor().idx() &&
>> pit() == normalAnchor().pit() &&
>> pos() == normalAnchor().pos())
>> setSelection(false);
>
>
> I think like Richard that the comment is incorrect.
Should we remove the comment then?
>> Does someone have an idea to fix it there?
>
>
> Get rid of setSelection(bool). Use selHandle whenever possible.
>
> Set transient selection mode (M-x mark-on) and move with regular cursor
> commands. It will show you places when selection is suddenly reset. This is
> not good.
I have not looked at this yet.
> The patches that add extra checks all over the code are not a good idea. Our
> helper code should be good enough for what we want to do.
OK.
Scott
From 38aef8e8bce24982b24428411966b1e91050f7a7 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak <skost...@lyx.org>
Date: Mon, 23 Jun 2014 06:28:53 -0400
Subject: [PATCH] Return _selection = true only if non-empty
Empty selections can cause confusing behavior for a few reasons:
(1) some functions behave differently depending on whether there is a
selection. If I press delete, nothing happens (where I expect the
character or inset before the cusor to be deleted). If I toggle bold or
emphasize nothing happens (where if there is no selection the entire
word is toggled). There are other LyX functions that depend on whether
there is a selection or not. Further, I wonder if any part of LyX's code
assumes that if there is a selection it is non-empty.
(2) menu options are incorrectly set. For example, the scissors icon.
---
src/Cursor.cpp | 14 ++++++++++++++
src/Cursor.h | 4 ++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index c740315..376de55 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -1138,6 +1138,20 @@ CursorSlice Cursor::normalAnchor() const
}
+bool Cursor::selection() const
+{
+ if (!selection_)
+ return false;
+
+ CursorSlice normal = anchor_[depth() - 1];
+ if (depth() < anchor_.depth() && top() <= normal) {
+ // anchor is behind cursor -> move anchor behind the inset
+ ++normal.pos();
+ }
+ return normal != top();
+}
+
+
DocIterator & Cursor::realAnchor()
{
return anchor_;
diff --git a/src/Cursor.h b/src/Cursor.h
index c3ae448..bdf9c83 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -162,8 +162,8 @@ public:
//
// selection
//
- /// selection active?
- bool selection() const { return selection_; }
+ /// selection active and non-empty?
+ bool selection() const;
/// set selection;
void setSelection(bool sel) { selection_ = sel; }
/// do we have a multicell selection?
--
1.8.3.2
