address comments

qgis · Jan 31, 2022 · 6b64270 · 6b64270
1 parent 3105c5e
commit 6b64270
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 4 deletions.
diff --git a/resources/function_help/json/array_remove_at b/resources/function_help/json/array_remove_at
@@ -2,9 +2,9 @@
   "name": "array_remove_at",
   "type": "function",
   "groups": ["Arrays"],
-  "description": "Returns an array with the item at the given index remove. Support positive (0 for the first element) and negative( the last -Nth value, -1 for the last element) index.",
+  "description": "Returns an array with the item at the given index remove. Supports positive (0 for the first element) and negative (the last -Nth value, -1 for the last element) index.",
   "arguments": [ {"arg":"array","description":"an array"},
                  {"arg":"pos","description":"the position to remove (0 based)"}],
-  "examples": [ { "expression":"array_remove_at(array(1,2,3),1)", "returns":"[ 1, 3 ]"},
-    { "expression":"array_remove_at(array(1,2,3),-1)", "returns":"[ 1, 2 ]"}]
+  "examples": [ { "expression":"array_remove_at(array(1, 2, 3), 1)", "returns":"[1, 3 ]"},
+    { "expression":"array_remove_at(array(1, 2, 3), -1)", "returns":"[1, 2 ]"}]
 }
diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp
@@ -6193,7 +6193,7 @@ static QVariant fcnArrayRemoveAt( const QVariantList &values, const QgsExpressio
   int position = QgsExpressionUtils::getNativeIntValue( values.at( 1 ), parent );
   if ( position < 0 && ( list.length() + position ) >= 0 )
     position = position + list.length();
-  if ( position < list.length() )
+  if ( position >= 0 && position < list.length() )
     list.removeAt( position );
   return convertToSameType( list, values.at( 0 ).type() );
 }

diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -3751,6 +3751,7 @@ class TestQgsExpression: public QObject
       QCOMPARE( QgsExpression( "array_remove_at(\"strings\", -2)" ).evaluate( &context ), QVariant( removeAtExpected ) );
       QCOMPARE( QgsExpression( "array_remove_at(\"strings\", -4)" ).evaluate( &context ), QVariant( array ) );
       QCOMPARE( QgsExpression( "array_remove_at(\"strings\", 4)" ).evaluate( &context ), QVariant( array ) );
+      QCOMPARE( QgsExpression( "array_remove_at(\"strings\", -40)" ).evaluate( &context ), QVariant( array ) );
 
       QStringList removeAllExpected;
       removeAllExpected << QStringLiteral( "a" ) << QStringLiteral( "b" ) << QStringLiteral( "d" );
@@ -3838,6 +3839,7 @@ class TestQgsExpression: public QObject
       QCOMPARE( QgsExpression( "array_remove_at(\"ints\", -2)" ).evaluate( &context ), QVariant( removeAtExpected ) );
       QCOMPARE( QgsExpression( "array_remove_at(\"ints\", -5)" ).evaluate( &context ), QVariant( array ) );
       QCOMPARE( QgsExpression( "array_remove_at(\"ints\", 5)" ).evaluate( &context ), QVariant( array ) );
+      QCOMPARE( QgsExpression( "array_remove_at(\"ints\", -50)" ).evaluate( &context ), QVariant( array ) );
 
       QVariantList removeAllExpected;
       removeAllExpected << 1 << 2 << 4;