need for kateHighlight::isString( attrib )?

add author to CC, hope he can help.

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

Note that bug#108951 has a patch for the colon-in-quotes behaviour, but as Tim points out, it's better to solve this more generally. However, I don't think Tim's suggestion is sufficient; it doesn't cope with goto-labels. It is better than what we've got at the moment in most cases where the outcome will be different, though, so I may put together a patch implementing that behaviour (but don't consider this a promise, or hold your breath, or anything like that).

By the way, S&S is described here:

http://www.derkarl.org/s-and-s.html

Oh yeah, forgot about gotos! (I never use them...).

How about: Indent (by which I mean unindent) if the text before the colon contains only valid label characters and whitespace, but not *just* whitespace:

foo::foo()
    : dont_want_to_indent_this(1)
{
only_has_valid_label_chars:
    string foo = "This works: Has an = and a \" before the :";
}

This would also automatically work for signals, slots, private slots, etc.

This bug is annoying me quite a lot because of these sort of statements:

    cerr << "Error: Couldn't open file." << endl;

If you can tell me the appropriate place in kate to make the change, and roughly what to do then I'll give it a go.

I found the place, and am going to implement it with one minor alteration - don't indent if the text before contains 'class'.

This still isn't 100% correct.

Now this would work properly:

namespace Trolltech
{
    class QWidget : public QObject
    {
    };
}

But any labels with 'class' in them wouldn't:

void f()
{
****class_start:
    int i = 10;
    goto class_start;
}

However I believe the second case is far less common.

Also, I just build KDE 4 kdelibs/base to test it in - should have gone with KDE 3 as indentation is pretty broken in general in KDE 4!

And it goes a little something like ... this:

bool KateCSAndSIndent::startsWithLabel( int line )
{
  // Get the current line.
  KateTextLine::Ptr indentLine = doc->plainKateTextLine(line);
  const int indentFirst = indentLine->firstChar();

  // Not entirely sure what this check does.
  int attrib = indentLine->attribute(indentFirst);
  if (attrib != 0 && attrib != keywordAttrib && attrib != normalAttrib && attrib != extensionAttrib)
    return false;

  // Get the line text.
  const QString lineContents = indentLine->string();
  const int indentLast = indentLine->lastChar();
  for ( int n = indentFirst; n <= indentLast; ++n )
  {
    // Get the character as latin1. Can't use QChar::isLetterOrNumber() as that includes non 0-9 numbers.
    char c = lineContents[n].toLatin1();
    if ( c == ':' )
    {
      // See if the next character is ':' - if so, skip to the character after it.
      if ( n < lineContents.length() - 1 )
      {
        if ( lineContents[n+1].toLatin1() == ':' )
        {
          n += 2;
          continue;
        }
      }
      // Right this is the relevent ':'.
      if ( n != indentFirst && !lineContents.left(n).contains("class") )
      {
        // It is a label of some kind!
        return true;
      }
      // Just a line with a : on it, or it has 'class' before it.
      return false;
    }
    // All other characters don't indent.
    if ( !isalnum(c) && c != '_' && !isspace(c) )
    {
      return false;
    }
  }
  return false;
}

I've tested it with kate from KDE 4 (as much as I can). Almost definitely works perfectly with KDE 3.

If someone says "yes", I shall:

a) Try to remember my SVN password.
b) Give up and ask for a new one.
c) Apply this to 3.5.5 and trunk.
d) Mark as fixed.

Tim:
Can't you test if 'class' is separated by delimiters? It seems like that 
would fix at least your example label (not sure if 'class' itself is a 
valid label, but even if it is, I don't think that will be used often if 
ever).

Well there are the following class cases:

   class QWidget : QObject

   label_class:

   label_class :

   classy_label:

So after thinking I will change it so that if the first whitespace delimited 'word' is 'class', then it returns false. Which makes all of the above cases work (I think).

Haven't compiled/tested this yet:

bool KateCSAndSIndent::startsWithLabel( int line ) 
{ 
   // Get the current line. 
   KateTextLine::Ptr indentLine = doc->plainKateTextLine(line); 
   const int indentFirst = indentLine->firstChar(); 
 
   // Not entirely sure what this check does. 
   int attrib = indentLine->attribute(indentFirst); 
   if (attrib != 0 && attrib != keywordAttrib && attrib != normalAttrib && attrib != extensionAttrib) 
     return false; 
 
   // Get the line text. 
   const QString lineContents = indentLine->string();
   const int indentLast = indentLine->lastChar();
   bool whitespaceFound = false;
   for ( int n = indentFirst; n <= indentLast; ++n ) 
   { 
     // Get the character as latin1. Can't use QChar::isLetterOrNumber() as that includes non 0-9 numbers. 
     char c = lineContents[n].toLatin1(); 
     if ( c == ':' ) 
     { 
       // See if the next character is ':' - if so, skip to the character after it. 
       if ( n < lineContents.length() - 1 ) 
       { 
         if ( lineContents[n+1].toLatin1() == ':' ) 
         { 
           n += 2; 
           continue; 
         }
       }
       // Right this is the relevent ':'. 
       if ( n == indentFirst)
       { 
         // Just a line with a : on it.
         return false;
       } 
       // It is a label of some kind! 
       return true;
     }
     if (isspace(c))
     {
       if (!whitespaceFound)
       {
         if (lineContents.mid(indentFirst, n - indentFirst) == "class")
           return false;
         whitespaceFound = true;
       }
     }
     // All other characters don't indent. 
     else if ( !isalnum(c) && c != '_' )
     { 
       return false; 
     } 
   } 
   return false; 
} 

Ok I have tested it with KDE 3.5 (just need to change 'toLatin1()' to 'latin1()', and an #include <cctype>).

It works with every case I can think to try.

Ok to commit?

SVN commit 576681 by thutt:

BUG: 113033

Fix S&S indentation for colons.

 M  +47 -29    kateautoindent.cpp  


--- branches/KDE/3.5/kdelibs/kate/part/kateautoindent.cpp #576680:576681
@@ -31,6 +31,8 @@
 #include <kdebug.h>
 #include <kpopupmenu.h>
 
+#include <cctype>
+
 //BEGIN KateAutoIndent
 
 KateAutoIndent *KateAutoIndent::createIndenter (KateDocument *doc, uint mode)
@@ -1552,44 +1554,60 @@
  * Does the line @p line start with a label?
  * @note May also return @c true if the line starts in a continuation.
  */
-bool KateCSAndSIndent::startsWithLabel( int line )
-{
-  KateTextLine::Ptr indentLine = doc->plainKateTextLine( line );
+bool KateCSAndSIndent::startsWithLabel( int line ) 
+{ 
+  // Get the current line.
+  KateTextLine::Ptr indentLine = doc->plainKateTextLine(line);
   const int indentFirst = indentLine->firstChar();
-
+  
+  // Not entirely sure what this check does.
   int attrib = indentLine->attribute(indentFirst);
   if (attrib != 0 && attrib != keywordAttrib && attrib != normalAttrib && attrib != extensionAttrib)
     return false;
-
+  
+  // Get the line text.
   const QString lineContents = indentLine->string();
-  static const QString symbols = QString::fromLatin1(";:[]{}");
-  const int last = indentLine->lastChar();
-  for ( int n = indentFirst + 1; n <= last; ++n )
+  const int indentLast = indentLine->lastChar();
+  bool whitespaceFound = false;
+  for ( int n = indentFirst; n <= indentLast; ++n )
   {
-    QChar c = lineContents[n];
-    // FIXME: symbols inside comments are not skipped
-    if ( !symbols.contains(c) )
-      continue;
-
-    // if we find a symbol other than a :, this is not a label.
-    if ( c != ':' )
-      return false;
-
-    // : but not ::, this is a label.
-    if ( lineContents[n+1] != ':' )
+    // Get the character as latin1. Can't use QChar::isLetterOrNumber()
+    // as that includes non 0-9 numbers.
+    char c = lineContents[n].latin1();
+    if ( c == ':' )
+    {
+      // See if the next character is ':' - if so, skip to the character after it.
+      if ( n < lineContents.length() - 1 )
+      {
+        if ( lineContents[n+1].latin1() == ':' )
+        {
+          n += 2;
+          continue;
+        }
+      }
+      // Right this is the relevent ':'.
+      if ( n == indentFirst)
+      {
+        // Just a line with a : on it.
+        return false;
+      }
+      // It is a label of some kind!
       return true;
-
-    // xy::[^:] is a scope-resolution operator. can occur in case X::Y: for instance.
-    // skip both :s and keep going.
-    if ( lineContents[n+2] != ':' )
+    }
+    if (isspace(c))
     {
-      ++n;
-      continue;
+      if (!whitespaceFound)
+      {
+        if (lineContents.mid(indentFirst, n - indentFirst) == "class")
+          return false;
+        whitespaceFound = true;
+      }
     }
-
-    // xy::: outside a continuation is a label followed by a scope-resolution operator.
-    // more than 3 :s is illegal, so we don't care that's not indented.
-    return true;
+    // All other characters don't indent.
+    else if ( !isalnum(c) && c != '_' )
+    {
+      return false;
+    }
   }
   return false;
 }

Tim, please do not forget to port this to trunk, and CCBUG this bug so we know this really went into trunk.

Ah yes, sorry!

Will do.

SVN commit 577863 by thutt:

CCBUG: 113033

Fix "case ':':" and similar cases.

 M  +3 -1      kateautoindent.cpp  


--- branches/KDE/3.5/kdelibs/kate/part/kateautoindent.cpp #577862:577863
@@ -1601,7 +1601,9 @@
     {
       if (!whitespaceFound)
       {
-        if (lineContents.mid(indentFirst, n - indentFirst) == "class")
+        if (lineContents.mid(indentFirst, n - indentFirst) == "case")
+          return true;
+        else if (lineContents.mid(indentFirst, n - indentFirst) == "class")
           return false;
         whitespaceFound = true;
       }

SVN commit 577948 by thutt:

CCBUG: 113033

Fix in KDE 4.

 M  +50 -27    kateautoindent.cpp  


--- trunk/KDE/kdelibs/kate/part/kateautoindent.cpp #577947:577948
@@ -33,6 +33,8 @@
 #include <kdebug.h>
 #include <kmenu.h>
 
+#include <cctype>
+
 //BEGIN KateAutoIndent
 
 KateAutoIndent *KateAutoIndent::createIndenter (KateDocument *doc, const QString &name)
@@ -1641,42 +1643,63 @@
  */
 bool KateCSAndSIndent::startsWithLabel( int line )
 {
-  KateTextLine::Ptr indentLine = doc->plainKateTextLine( line );
+  // Get the current line.
+  KateTextLine::Ptr indentLine = doc->plainKateTextLine(line);
   const int indentFirst = indentLine->firstChar();
-
+  
+  // This kind of messes up Q_OBJECT\n public: but I'll leave it for now.
+  // Otherwise it messes up other cases.
   int attrib = indentLine->attribute(indentFirst);
   if (attrib != 0 && attrib != keywordAttrib && attrib != normalAttrib && attrib != extensionAttrib)
     return false;
+  
+  // Get the line text.
+  const QString lineContents = indentLine->string();
+  const int indentLast = indentLine->lastChar();
+  bool whitespaceFound = false;
 
-  const QString lineContents = indentLine->string();
-  static const QString symbols = QLatin1String(";:[]{}");
-  const int last = indentLine->lastChar();
-  for ( int n = indentFirst + 1; n <= last; ++n )
+  // Extra checks needed as indentFirst can be -1 in KDE4.
+  for ( int n = indentFirst; n <= indentLast && n >= 0 && n < lineContents.length(); ++n )
   {
-    QChar c = lineContents[n];
-    // FIXME: symbols inside comments are not skipped
-    if ( !symbols.contains(c) )
-      continue;
-
-    // if we find a symbol other than a :, this is not a label.
-    if ( c != ':' )
-      return false;
-
-    // : but not ::, this is a label.
-    if ( lineContents[n+1] != ':' )
+    // Get the character as latin1. Can't use QChar::isLetterOrNumber()
+    // as that includes non 0-9 numbers.
+    char c = lineContents[n].toLatin1();
+    if ( c == ':' )
+    {
+      // See if the next character is ':' - if so, skip to the character after it.
+      if ( n < lineContents.length() - 1 )
+      {
+        if ( lineContents[n+1].toLatin1() == ':' )
+        {
+          n += 2;
+          continue;
+        }
+      }
+      // Right this is the relevent ':'.
+      if ( n == indentFirst)
+      {
+        // Just a line with a : on it.
+        return false;
+      }
+      // It is a label of some kind!
       return true;
-
-    // xy::[^:] is a scope-resolution operator. can occur in case X::Y: for instance.
-    // skip both :s and keep going.
-    if ( lineContents[n+2] != ':' )
+    }
+    if (isspace(c))
     {
-      ++n;
-      continue;
+      if (!whitespaceFound)
+      {
+        if (lineContents.mid(indentFirst, n - indentFirst) == "case")
+          return true;
+        else if (lineContents.mid(indentFirst, n - indentFirst) == "class")
+          return false;
+        whitespaceFound = true;
+      }
     }
-
-    // xy::: outside a continuation is a label followed by a scope-resolution operator.
-    // more than 3 :s is illegal, so we don't care that's not indented.
-    return true;
+    // All other characters don't indent.
+    else if ( !isalnum(c) && c != '_' )
+    {
+      return false;
+    }
   }
   return false;
 }