Bug 133937

I can also reproduce this by typing new text instead of pasting (step 4).

Hello? Is anyone able to reproduce this?

On Wednesday 08 November 2006 20:34, Matthew Woehlke wrote:
> Hello? Is anyone able to reproduce this?

Not me.
And I disagree. At least the current version of kate, which does not work 
exactly as you describe, is very consistent, but not working like you 
suggest.

Ctrl + arrow moves the selection end one word in the desired correction, so 
that the next word becomes selected, or deselected if the direction is from 
the selection end towards the selection start.

I can kind of reproduce. The selection is extended to the left, although you only go to the right. I'm not sure if that's how it is supposed to work.

Do you mind giving me a complete description (like what I did) of what you are seeing? I'd be very interested in comparing notes.

Anyway, if you have any ideas, I'd be more than happy to test patches.

I should add that in 3.5.5, 5 and 6 are now:
5: foo bar[     ]|*/
6: foo |bar[     ]*/

...and the selection at 6 is clearly wrong; the cursor is completely outside the selection, which AFAIK is not supposed to be possible.

Actually, I think I know what the problem is... when you double-click to select a word, the cursor position becomes decoupled from both the selection start and end. While I can see where this *might* have been an intentional design, it causes behavior that is at best described as "strange". To demonstrate, type a long word surrounded by other text (just so you have somewhere to move the cursor to), and double-click it in the middle. The selection start and end become the word, while the cursor is in the middle of the word. From here, what happens as you shift-arrow off of the word always leaves the word selected. Again, while I can see this as being "desirable", it is not intuitive (I would describe it as "strange, at best") and is inconsistent with how double-click selection works in other circumstances (QLineEdit, for example, or this firefox text box I am typing in), and is something I have never found useful. Having discovered this, I suspect it is the cause of this bug, which crops up often enough to be quite irritating.

What SHOULD be happening here (and does anywhere outside of KATE AFAICT) is that double-clicking sets the cursor - equal to the selection end - to the end of the word, rather than the click location. If you then shift-left from here, you will wind up with only part of the word selected.

Uhm, I see that you are in fact correct, this does happen.

The problem is during the paste. Paste clears the selection, but not the 
selectAnchor property used internally i guess, this should not be too hard to 
fix.

> Actually, I think I know what the problem is... when you double-click to 
> select a word, the cursor position becomes decoupled from both the selection 
> start and end. While I can see where this *might* have been an intentional 
> design, it causes behavior that is at best described as "strange". To 
> demonstrate, type a long word surrounded by other text (just so you have 
> somewhere to move the cursor to), and double-click it in the middle.

double click -> now you are in the select-whole-words-only mode

> The selection start and end become the word, while the cursor is in the
> middle of the word. From here, what happens as you shift-arrow off of the
> word always leaves the word selected. Again, while I can see this as 
> being "desirable", it is not intuitive (I would describe it as "strange, at
> best") and is inconsistent with how double-click selection works in other 
> circumstances (QLineEdit, for example, or this firefox text box I am typing 
> in), and is something I have never found useful. Having discovered this, I 
> suspect it is the cause of this bug, which crops up often enough to be quite 
> irritating.

Our standard documents state that double clicking results in the select-words-only mode. see http://developer.kde.org/documentation/standards/kde/style/mouse/selection.html
Maybe using the keyboard afterwards should behave differently (as you are not using the mouse then).

> What SHOULD be happening here (and does anywhere outside of KATE AFAICT) is
> that double-clicking sets the cursor - equal to the selection end - to the 
> end of the word, rather than the click location. If you then shift-left from 
> here, you will wind up with only part of the word selected. 

This was changed to fix bug #106402.

(sorry if this double-posts, after a few hours I'm giving up on <id> AT bugs-kde-org actually posting...)

> double click -> now you are in the select-whole-words-only mode
> [snip]
> Our standard documents state that double clicking results in the
> select-words-only mode. see
> http://developer.kde.org/documentation/standards/kde/style/mouse/selection.html
> Maybe using the keyboard afterwards should behave differently (as you are not
> using the mouse then).

Yes and no. There is no reason 'select-words-only' mode needs to decouple the cursor from the selection-end. What IMO should happen is the cursor follows the selection end rather than the position of the mouse click. Again, this is consistent with other editors, and would result in no further problems once you release the mouse button (which should end words-only selection mode also). It looks like WOSM ends when it should, except that the select start/end are already broken.

> This was changed to fix bug #106402.

I don't see how that precludes making select work as I describe, i.e. the way it works in /every/other/editor/.

Here is what I think should happen for successive clicks:
1. clears selection, sets cursor to position of click
2. selects word under cursor; cursor is at right of word (selection end)
3. selects entire line; cursor still equal to selection end, at end of line
4. Same as 2

...and I'm not sure what should happen at 5 and beyond; behavior right now seems to be "touchy" (i.e. really sensitive to speed and timing), but I think at this point all that matters is not breaking anything.

I tried playing with this but didn't get far (fixing the cursor placement when multi-clicking is easy, but unfortunately doesn't fix any of the weirdness). I still think what needs to happen is that this weird 'preserving the selected word' stuff needs to go away. Double-clicking should set the select start and end, and set the cursor as described. After that, the select start should remain constant and the select end should stay glued to the cursor.

Replying to comment #6: I (finally) figured out what Anders is talking about with the original report. Now that I'm starting to understand how KATE handles selections, I agree in principle, although I'm not sure it is selectAnchor (isn't this *always* set?) as opposed to sel*Cached that is not purged correctly. Especially since I believe that they *do* need to be purged on paste, and I didn't yet spot where this should be (and I suspect, isn't) happening.

I also finally found how to kill the KATE-unique behavior talked about in comment #8, and it doesn't fix the bug, so comment #8 is talking about a different issue.

Ok, lets fight this until we get to a point where we are hopefully all good :)

hm, i didn't mean to do that, let's keep it on the list!

Indeed. We will do that (making this comment for the record).

SVN commit 633268 by mwoehlke:

BUG: 133937

Update selection after paste. Also, don't preserve the word from a double-click selection when selecting with the keyboard.


 M  +7 -0      kateview.cpp  
 M  +1 -1      kateview.h  
 M  +7 -13     kateviewinternal.cpp  


--- branches/KDE/3.5/kdelibs/kate/part/kateview.cpp #633267:633268
@@ -1592,6 +1592,13 @@
   setSelection (cursor.line(), start, cursor.line(), end);
 }
 
+void KateView::paste()
+{
+  m_doc->paste( this );
+  emit selectionChanged();
+  m_viewInternal->repaint();
+}
+
 void KateView::cut()
 {
   if (!hasSelection())
--- branches/KDE/3.5/kdelibs/kate/part/kateview.h #633267:633268
@@ -83,7 +83,7 @@
   //
   public slots:
     // TODO: Factor out of m_viewInternal
-    void paste()         {  m_doc->paste( this ); m_viewInternal->repaint(); }
+    void paste();
     void cut();
     void copy() const;
 
--- branches/KDE/3.5/kdelibs/kate/part/kateviewinternal.cpp #633267:633268
@@ -2097,23 +2097,13 @@
           else // same line, ignore
             doSelect = false;
         break;
-        default: // *allways* keep original selection for mouse
+        default:
         {
           if ( selStartCached.line() < 0 ) // invalid
             break;
 
-          if ( newCursor.line() > selEndCached.line() ||
-               ( newCursor.line() == selEndCached.line() &&
-                 newCursor.col() > selEndCached.col() ) )
-            selectAnchor = selStartCached;
-
-          else if ( newCursor.line() < selStartCached.line() ||
-               ( newCursor.line() == selStartCached.line() &&
-                 newCursor.col() < selStartCached.col() ) )
-            selectAnchor = selEndCached;
-
-          else
-            doSelect = false;
+          selectAnchor = selStartCached;
+          doSelect = true;
         }
 //         break;
       }
@@ -3264,7 +3254,11 @@
 void KateViewInternal::viewSelectionChanged ()
 {
   if (!m_view->hasSelection())
+  {
     selectAnchor.setPos (-1, -1);
+    selEndCached = selectAnchor;
+    selStartCached = cursor;
+  }
 }
 
 //BEGIN IM INPUT STUFF

SVN commit 655507 by mwoehlke:

CCBUG: 133937
Fixing 133937 broke mouse selection which *should* preserve the selected word. So restore the behavior removed by r633268, but make it conditional on mouse selection (new selection mode) so that both will Do The Right Thing.


 M  +21 -3     kateviewinternal.cpp  
 M  +2 -2      kateviewinternal.h  


--- branches/KDE/3.5/kdelibs/kate/part/kateviewinternal.cpp #655506:655507
@@ -2115,14 +2115,24 @@
           else // same line, ignore
             doSelect = false;
         break;
+        case Mouse:
+        {
+          if ( selStartCached.line() < 0 ) // invalid
+            break;
+
+          if ( newCursor > selEndCached )
+            selectAnchor = selStartCached;
+          else if ( newCursor < selStartCached )
+            selectAnchor = selEndCached;
+          else
+            doSelect = false;
+        }
+        break;
         default:
         {
           if ( selectAnchor.line() < 0 ) // invalid
             break;
-
-          doSelect = true;
         }
-//         break;
       }
 
       if ( doSelect )
@@ -2687,6 +2697,10 @@
           e->accept ();
           return;
         }
+        else if (m_selectionMode == Default)
+        {
+          m_selectionMode = Mouse;
+        }
 
         if ( e->state() & Qt::ShiftButton )
         {
@@ -2802,6 +2816,10 @@
 
         selStartCached = m_view->selectStart;
         selEndCached = m_view->selectEnd;
+        // FIXME this isn't right in the case of a shift+DC
+        // for shift+DC, we might want to use selStartCached!!
+        // This also breaks shift+TC because it nukes the cursor
+        // order for when the TC handler does its thing. Oops.
         updateCursor( selEndCached );
       }
 
--- branches/KDE/3.5/kdelibs/kate/part/kateviewinternal.h #655506:655507
@@ -273,7 +273,7 @@
     bool m_selChangedByUser;
     KateTextCursor selectAnchor;
 
-    enum SelectionMode { Default=0, Word, Line }; ///< for drag selection. @since 2.3
+    enum SelectionMode { Default=0, Word, Line, Mouse }; ///< for drag selection. @since 2.3
     uint m_selectionMode;
     // when drag selecting after double/triple click, keep the initial selected
     // word/line independant of direction.
@@ -288,7 +288,7 @@
 
     // maximal lenght of textlines visible from given startLine
     int maxLen(uint startLine);
-    
+
     // are we allowed to scroll columns?
     bool columnScrollingPossible ();
 

*** Bug 146673 has been marked as a duplicate of this bug. ***