I agree, I think we should stop at anything matching the \b regexp assertion. Should we 
make it configurable? 

I don't think this requires a configuration option, in fact configuring how
Ctrl+arrows work would definitely fall into the realms of "geeky
hyperconfigurability" in my book! The most important thing is consistency
between moving left and right, followed by consistency with the rest of KDE, IMHO.

yeah, please change it to the \b thingy without config option, that is what the rest of 
kde/qt is used to do, should be fine for us, too  

works in CVS 

This no longer works as advertised in SUSE 9.3 (KDE 3.4.0, Kate 2.4). Consider again:

foo => bar

Starting right before foo, the cursor jumps forward right before =>, then before bar, then to the end of the line. But backwards it jumps before bar and then immediately before foo, and not => as I would expect.

-- Hannes

Created attachment 16444 [details]
makes wordLeft() and wordRight() behavior symmetric

This patch fixes that word moves forward and backward (Ctrl-Left and
Ctrl-Right) are not exactly symmetric, i.e. they do not stop at exactly the
same points.

The previous implementation unifies wordLeft() and wordRight() into a single
function wordMove(), but the problem is that the move operations are such
that they must be implemented assymetrically, and cannot really be unified.
The patch does right this: It splits the implementation into two: one for
wordLeft() and wordRight() each.

The behavior with this patch is this:

wordRight():
- If the cursor is on a word character, the word and space following it is
skipped.
- If the cursor is on a non-word character, all following non-word characters
excluding space are skipped, then all space following it is skipped.
- If the cursor is at the end of the line, all space at the beginning of the
next line is skipped. It does not skip forward for more than one line.

wordLeft():
All space before the cursor is skipped. Then:
- If there is a word character before the cursor, it is skipped.
- If there is a non-word character before the cursor, all non-word characters
excluding space are skipped.
- If the cursor it is at the beginning of the line, it is moved to the end of
the previous line.

This behavior is now symmetric. It _is_ different from the word move
operations of some other edit widgets (like, for example, KMail's composer),
but it is more appropriate for a programmer's editor, because it stops at all
punctuation.

Imho the suggested patch is fine.
Christoph, Anders, Hamish? :)

FWIW, in my opinion that is very desirable ;)

-anders

Am Samstag, 3. Juni 2006 15:25 schrieb Johannes Sixt:
> The behavior with this patch is this:


Just a small question for clarification (not having tested the patch).

On the snippet below where | denotes the position of the caret, does 
sequentially pressing of Ctrl+Right result in the following movement?

1. |some.identifier, .some.other
2. some.|identifier, .some.other
3. some.identifier, |.some.other
4. some.identifier, .some.|other
5. some.identifier, .some.other|

which I think is good enough for not moving too slowly and not getting too 
much in your way.

Or does it result in
1. |some.identifier, .some.other
2. some.identifier, |.some.other
3. some.identifier, .some.other|
4. some.identifier, .some.other

or does it result in
1. |some.identifier, .some.other
2. some|.identifier, .some.other
3. some.|identifier, .some.other
4. some.identifier|, .some.other
5. some.identifier,| .some.other
6. some.identifier, |.some.other
7. some.identifier, .|some.other
8. some.identifier, .some|.other
etc.

It works like this:

1. |some.identi_fier, some-> other
2. some|.identi_fier, some-> other (*)
3. some.|identi_fier, some-> other
4. some.identi_fier|, some-> other (*)
5. some.identi_fier, |some-> other
6. some.identi_fier, some|-> other (*)
7. some.identi_fier, some-> |other
8. some.identi_fier, some-> other|

Currently, Ctrl-Right does right this, but Ctrl-Left skips these (*). The patch fixes that.

I'd like to commit this on the 3.5 branch (and later on the trunk). Any objections?

If you tested it well, then I have none ;)

SVN commit 550059 by jsixt:

Make wordLeft() and wordRight() behavior symmetric.

Consider this line:

  if (num >= 0) num--; else return;

The points where the cursor stops are (indicated by |):

  |if |(|num |>= |0|) |num|--; |else |return|;|

with both Ctrl-Left and Ctrl-Right.

This behavior _is_ different from the word move operations of some other
edit widgets (like, for example, KMail's composer), but it is more
appropriate for a programmer's editor, because it stops at all punctuation.
BUG: 56337


 M  +70 -23    kateviewinternal.cpp  
 M  +0 -1      kateviewinternal.h  


--- branches/KDE/3.5/kdelibs/kate/part/kateviewinternal.cpp #550058:550059
@@ -1123,43 +1123,90 @@
   }
 }
 
-void KateViewInternal::moveWord( Bias bias, bool sel )
+void KateViewInternal::wordLeft ( bool sel )
 {
-  // This matches the word-moving in QTextEdit, QLineEdit etc.
-
   WrappingCursor c( this, cursor );
-  if( !c.atEdge( bias ) ) {
-    KateHighlighting* h = m_doc->highlight();
 
-    bool moved = false;
-    while( !c.atEdge( bias ) && !h->isInWord( m_doc->textLine( c.line() )[ c.col() - (bias == left ? 1 : 0) ] ) )
+  // First we skip backwards all space.
+  // Then we look up into which category the current position falls:
+  // 1. a "word" character
+  // 2. a "non-word" character (except space)
+  // 3. the beginning of the line
+  // and skip all preceding characters that fall into this class.
+  // The code assumes that space is never part of the word character class.
+
+  KateHighlighting* h = m_doc->highlight();
+  if( !c.atEdge( left ) ) {
+
+    while( !c.atEdge( left ) && m_doc->textLine( c.line() )[ c.col() - 1 ].isSpace() )
+      --c;
+  }
+  if( c.atEdge( left ) )
+  {
+    --c;
+  }
+  else if( h->isInWord( m_doc->textLine( c.line() )[ c.col() - 1 ] ) )
+  {
+    while( !c.atEdge( left ) && h->isInWord( m_doc->textLine( c.line() )[ c.col() - 1 ] ) )
+      --c;
+  }
+  else
+  {
+    while( !c.atEdge( left )
+           && !h->isInWord( m_doc->textLine( c.line() )[ c.col() - 1 ] )
+           // in order to stay symmetric to wordLeft()
+           // we must not skip space preceding a non-word sequence
+           && !m_doc->textLine( c.line() )[ c.col() - 1 ].isSpace() )
     {
-      c += bias;
-      moved = true;
+      --c;
     }
+  }
 
-    if ( bias != right || !moved )
+  updateSelection( c, sel );
+  updateCursor( c );
+}
+
+void KateViewInternal::wordRight( bool sel )
+{
+  WrappingCursor c( this, cursor );
+
+  // We look up into which category the current position falls:
+  // 1. a "word" character
+  // 2. a "non-word" character (except space)
+  // 3. the end of the line
+  // and skip all following characters that fall into this class.
+  // If the skipped characters are followed by space, we skip that too.
+  // The code assumes that space is never part of the word character class.
+
+  KateHighlighting* h = m_doc->highlight();
+  if( c.atEdge( right ) )
+  {
+    ++c;
+  }
+  else if( h->isInWord( m_doc->textLine( c.line() )[ c.col() ] ) )
+  {
+    while( !c.atEdge( right ) && h->isInWord( m_doc->textLine( c.line() )[ c.col() ] ) )
+      ++c;
+  }
+  else
+  {
+    while( !c.atEdge( right )
+           && !h->isInWord( m_doc->textLine( c.line() )[ c.col() ] )
+           // we must not skip space, because if that space is followed
+           // by more non-word characters, we would skip them, too
+           && !m_doc->textLine( c.line() )[ c.col() ].isSpace() )
     {
-      while( !c.atEdge( bias ) &&  h->isInWord( m_doc->textLine( c.line() )[ c.col() - (bias == left ? 1 : 0) ] ) )
-        c += bias;
-      if ( bias == right )
-      {
-        while ( !c.atEdge( bias ) && m_doc->textLine( c.line() )[ c.col() ].isSpace() )
-          c+= bias;
-      }
+      ++c;
     }
-
-  } else {
-    c += bias;
   }
 
+  while( !c.atEdge( right ) && m_doc->textLine( c.line() )[ c.col() ].isSpace() )
+    ++c;
+
   updateSelection( c, sel );
   updateCursor( c );
 }
 
-void KateViewInternal::wordLeft ( bool sel ) { moveWord( left,  sel ); }
-void KateViewInternal::wordRight( bool sel ) { moveWord( right, sel ); }
-
 void KateViewInternal::moveEdge( Bias bias, bool sel )
 {
   BoundedCursor c( this, cursor );
--- branches/KDE/3.5/kdelibs/kate/part/kateviewinternal.h #550058:550059
@@ -190,7 +190,6 @@
 
   private:
     void moveChar( Bias bias, bool sel );
-    void moveWord( Bias bias, bool sel );
     void moveEdge( Bias bias, bool sel );
     KateTextCursor maxStartPos(bool changed = false);
     void scrollPos(KateTextCursor& c, bool force = false, bool calledExternally = false);

SVN commit 551148 by jsixt:

Forward-port r550059: Make wordLeft() and wordRight() behavior symmetric.
CCBUG: 56337


 M  +70 -23    kateviewinternal.cpp  
 M  +0 -1      kateviewinternal.h  


--- trunk/KDE/kdelibs/kate/part/kateviewinternal.cpp #551147:551148
@@ -1013,43 +1013,90 @@
   }
 }
 
-void KateViewInternal::moveWord( KateViewInternal::Bias bias, bool sel )
+void KateViewInternal::wordLeft ( bool sel )
 {
-  // This matches the word-moving in QTextEdit, QLineEdit etc.
-
   WrappingCursor c( this, m_cursor );
-  if( !c.atEdge( bias ) ) {
-    KateHighlighting* h = m_doc->highlight();
 
-    bool moved = false;
-    while( !c.atEdge( bias ) && !h->isInWord( m_doc->line( c.line() )[ c.column() - (bias == left ? 1 : 0) ] ) )
+  // First we skip backwards all space.
+  // Then we look up into which category the current position falls:
+  // 1. a "word" character
+  // 2. a "non-word" character (except space)
+  // 3. the beginning of the line
+  // and skip all preceding characters that fall into this class.
+  // The code assumes that space is never part of the word character class.
+
+  KateHighlighting* h = m_doc->highlight();
+  if( !c.atEdge( left ) ) {
+
+    while( !c.atEdge( left ) && m_doc->line( c.line() )[ c.column() - 1 ].isSpace() )
+      --c;
+  }
+  if( c.atEdge( left ) )
+  {
+    --c;
+  }
+  else if( h->isInWord( m_doc->line( c.line() )[ c.column() - 1 ] ) )
+  {
+    while( !c.atEdge( left ) && h->isInWord( m_doc->line( c.line() )[ c.column() - 1 ] ) )
+      --c;
+  }
+  else
+  {
+    while( !c.atEdge( left )
+           && !h->isInWord( m_doc->line( c.line() )[ c.column() - 1 ] )
+           // in order to stay symmetric to wordLeft()
+           // we must not skip space preceding a non-word sequence
+           && !m_doc->line( c.line() )[ c.column() - 1 ].isSpace() )
     {
-      c += bias;
-      moved = true;
+      --c;
     }
+  }
 
-    if ( bias != right || !moved )
+  updateSelection( c, sel );
+  updateCursor( c );
+}
+
+void KateViewInternal::wordRight( bool sel )
+{
+  WrappingCursor c( this, m_cursor );
+
+  // We look up into which category the current position falls:
+  // 1. a "word" character
+  // 2. a "non-word" character (except space)
+  // 3. the end of the line
+  // and skip all following characters that fall into this class.
+  // If the skipped characters are followed by space, we skip that too.
+  // The code assumes that space is never part of the word character class.
+
+  KateHighlighting* h = m_doc->highlight();
+  if( c.atEdge( right ) )
+  {
+    ++c;
+  }
+  else if( h->isInWord( m_doc->line( c.line() )[ c.column() ] ) )
+  {
+    while( !c.atEdge( right ) && h->isInWord( m_doc->line( c.line() )[ c.column() ] ) )
+      ++c;
+  }
+  else
+  {
+    while( !c.atEdge( right )
+           && !h->isInWord( m_doc->line( c.line() )[ c.column() ] )
+           // we must not skip space, because if that space is followed
+           // by more non-word characters, we would skip them, too
+           && !m_doc->line( c.line() )[ c.column() ].isSpace() )
     {
-      while( !c.atEdge( bias ) &&  h->isInWord( m_doc->line( c.line() )[ c.column() - (bias == left ? 1 : 0) ] ) )
-        c += bias;
-      if ( bias == right )
-      {
-        while ( !c.atEdge( bias ) && m_doc->line( c.line() )[ c.column() ].isSpace() )
-          c+= bias;
-      }
+      ++c;
     }
-
-  } else {
-    c += bias;
   }
 
+  while( !c.atEdge( right ) && m_doc->line( c.line() )[ c.column() ].isSpace() )
+    ++c;
+
   updateSelection( c, sel );
   updateCursor( c );
 }
 
-void KateViewInternal::wordLeft ( bool sel ) { moveWord( left,  sel ); }
-void KateViewInternal::wordRight( bool sel ) { moveWord( right, sel ); }
-
 void KateViewInternal::moveEdge( KateViewInternal::Bias bias, bool sel )
 {
   BoundedCursor c( this, m_cursor );
--- trunk/KDE/kdelibs/kate/part/kateviewinternal.h #551147:551148
@@ -215,7 +215,6 @@
 
   private:
     void moveChar( Bias bias, bool sel );
-    void moveWord( Bias bias, bool sel );
     void moveEdge( Bias bias, bool sel );
     KTextEditor::Cursor maxStartPos(bool changed = false);
     void scrollPos(KTextEditor::Cursor& c, bool force = false, bool calledExternally = false);