[FEATURE] upgrade the substr() function

nirvn · nyalldawson · commit 0fbcc4b95d31 · 2016-11-08T08:42:17.000+10:00
- support negative start value (e.g. substr('hello',-2) returns 'lo')
- support negative length value (e.g. substr('hello',3,-1) returns 'll')
- length parameter now optional, defaults to end of string
  (e.g. substr('hello world',7) returns 'world')
diff --git a/resources/function_help/json/substr b/resources/function_help/json/substr
@@ -2,9 +2,15 @@
   "name": "substr",
   "type": "function",
   "description": "Returns a part of a string.",
-  "arguments": [ {"arg":"input_string","description":"the full input string"},
-  {"arg":"startpos","description":"integer representing start position to extract from"},
-  {"arg":"length","description":"integer representing length of string to extract"}
+  "arguments": [ {"arg":"string","description":"the full input string"},
+  {"arg":"start","description":"integer representing start position to extract from; if start is negative, the return string will begin at the end of the string minus the start value"},
+  {"arg":"length","optional":true,"description":"integer representing length of string to extract; if length is negative, the return string will omit the given length of characters from the end of the string"}
   ],
-  "examples": [ { "expression":"substr('HELLO WORLD',3,5)", "returns":"'LLO W'"}]
+  "examples": [ { "expression":"substr('HELLO WORLD',3,5)", "returns":"'LLO W'"},
+                { "expression":"substr('HELLO WORLD',6)", "returns":"'WORLD'"},
+                { "expression":"substr('HELLO WORLD',-5)","returns":"'WORLD'"},
+                { "expression":"substr('HELLO',3,-1)", "returns":"'LL'"},
+                { "expression":"substr('HELLO WORLD',-5,2)","returns":"'WO'"},
+                { "expression":"substr('HELLO WORLD',-5,-1)","returns":"'WORL'"}
+  ]
 }
diff --git a/src/core/qgsexpression.cpp b/src/core/qgsexpression.cpp
@@ -1398,12 +1398,55 @@ static QVariant fcnUuid( const QVariantList&, const QgsExpressionContext*, QgsEx
   return QUuid::createUuid().toString();
 }
 
-static QVariant fcnSubstr( const QVariantList& values, const QgsExpressionContext*, QgsExpression* parent )
+static QVariant fcnSubstr( const QVariantList& values, const QgsExpressionContext* context, QgsExpression* parent )
 {
-  QString str = getStringValue( values.at( 0 ), parent );
-  int from = getIntValue( values.at( 1 ), parent );
-  int len = getIntValue( values.at( 2 ), parent );
-  return QVariant( str.mid( from -1, len ) );
+
+  QgsExpression::Node* node;
+
+  node = getNode( values.at( 0 ), parent );
+  ENSURE_NO_EVAL_ERROR;
+  QString str = node->eval( parent, context ).toString();
+
+  node = getNode( values.at( 1 ), parent );
+  ENSURE_NO_EVAL_ERROR;
+  int from = node->eval( parent, context ).toInt();
+
+  node = getNode( values.at( 2 ), parent );
+  QVariant value = node->eval( parent, context );
+  int len;
+  if ( value.isValid() )
+  {
+    len = value.toInt();
+  }
+  else
+  {
+    len = str.size();
+  }
+
+  if ( from < 0 )
+  {
+    from = str.size() + from;
+    if ( from < 0 )
+    {
+      from = 0;
+    }
+  }
+  else if ( from > 0 )
+  {
+    //account for the fact that substr() starts at 1
+    from -= 1;
+  }
+
+  if ( len < 0 )
+  {
+    len = str.size() + len - from;
+    if ( len < 0 )
+    {
+      len = 0;
+    }
+  }
+
+  return QVariant( str.mid( from, len ) );
 }
 static QVariant fcnFeatureId( const QVariantList&, const QgsExpressionContext* context, QgsExpression* )
 {
@@ -3804,7 +3847,7 @@ const QList<QgsExpression::Function*>& QgsExpression::Functions()
     << new StaticFunction( QStringLiteral( "replace" ), -1, fcnReplace, QStringLiteral( "String" ) )
     << new StaticFunction( QStringLiteral( "regexp_replace" ), 3, fcnRegexpReplace, QStringLiteral( "String" ) )
     << new StaticFunction( QStringLiteral( "regexp_substr" ), 2, fcnRegexpSubstr, QStringLiteral( "String" ) )
-    << new StaticFunction( QStringLiteral( "substr" ), 3, fcnSubstr, QStringLiteral( "String" ) )
+    << new StaticFunction( QStringLiteral( "substr" ), ParameterList() << Parameter( QStringLiteral( "string" ) ) << Parameter( QStringLiteral( "start " ) ) << Parameter( QStringLiteral( "length" ), true ), fcnSubstr, QStringLiteral( "String" ), QString(), False, QSet<QString>(), true )
     << new StaticFunction( QStringLiteral( "concat" ), -1, fcnConcat, QStringLiteral( "String" ), QString(), false, QSet<QString>(), false, QStringList(), true )
     << new StaticFunction( QStringLiteral( "strpos" ), 2, fcnStrpos, QStringLiteral( "String" ) )
     << new StaticFunction( QStringLiteral( "left" ), 2, fcnLeft, QStringLiteral( "String" ) )
diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -836,7 +836,10 @@ class TestQgsExpression: public QObject
       QTest::newRow( "regexp_replace cap group" ) << "regexp_replace('HeLLo','(eL)', 'x\\\\1x')" << false << QVariant( "HxeLxLo" );
       QTest::newRow( "regexp_replace invalid" ) << "regexp_replace('HeLLo','[[[', '-')" << true << QVariant();
       QTest::newRow( "substr" ) << "substr('HeLLo', 3,2)" << false << QVariant( "LL" );
-      QTest::newRow( "substr outside" ) << "substr('HeLLo', -5,2)" << false << QVariant( "" );
+      QTest::newRow( "substr negative start" ) << "substr('HeLLo', -4)" << false << QVariant( "eLLo" );
+      QTest::newRow( "substr negative length" ) << "substr('HeLLo', 1,-3)" << false << QVariant( "He" );
+      QTest::newRow( "substr positive start and negative length" ) << "substr('HeLLo', 3,-1)" << false << QVariant( "LL" );
+      QTest::newRow( "substr start only" ) << "substr('HeLLo', 3)" << false << QVariant( "LLo" );
       QTest::newRow( "regexp_substr" ) << "regexp_substr('abc123','(\\\\d+)')" << false << QVariant( "123" );
       QTest::newRow( "regexp_substr non-greedy" ) << "regexp_substr('abc123','(\\\\d+?)')" << false << QVariant( "1" );
       QTest::newRow( "regexp_substr no hit" ) << "regexp_substr('abcdef','(\\\\d+)')" << false << QVariant( "" );
