Fix for longstanding bug where set -e would fail to erase elements from an array.

This was introduced in 7b3377e78c16b3ba74edb141ca0dd2ec895806b9

3 files changed, 22 insertions, 16 deletions

diff --git a/builtin_set.cpp b/builtin_set.cpp
index e2b3c2a2..d55421d8 100644
--- a/builtin_set.cpp
+++ b/builtin_set.cpp
@@ -275,23 +275,21 @@ static int update_values( wcstring_list_t &list,
 /**
    Erase from a list of wcstring values at specified indexes 
 */
-static void erase_values(wcstring_list_t &list, std::vector<long> &indexes) 
+static void erase_values(wcstring_list_t &list, const std::vector<long> &indexes) 
 {
-	size_t i;
-	wcstring_list_t result;
-	for (i = 0; i < list.size(); i++) 
-	{
-		if (std::find(indexes.begin(), indexes.end(),  i + 1) != indexes.end()) 
-		{
-			result.push_back( list[ i ] );
-		}
-	}
-	
-//	al_truncate(list,0);	
-	list.clear();
-	copy(result.begin(),result.end(),back_inserter( list ) );
-
-//	al_destroy(&result);
+    // Make a set of indexes.
+    // This both sorts them into ascending order and removes duplicates.
+    const std::set<long> indexes_set(indexes.begin(), indexes.end());
+    
+    // Now walk the set backwards, so we encounter larger indexes first, and remove elements at the given (1-based) indexes.
+    std::set<long>::const_reverse_iterator iter;
+    for (iter = indexes_set.rbegin(); iter != indexes_set.rend(); iter++) {
+        long val = *iter;
+        if (val > 0 && val <= list.size()) {
+            // One-based indexing!
+            list.erase(list.begin() + val - 1);
+        }
+    }
 }
 
 
diff --git a/tests/test3.in b/tests/test3.in
index c6801999..e1594683 100644
--- a/tests/test3.in
+++ b/tests/test3.in
@@ -127,3 +127,10 @@ else
 end
 
 
+set foo abc def
+set -e foo[1]
+if test $foo '=' def
+	echo Test 11 pass
+else
+	echo Test 11 fail
+end
diff --git a/tests/test3.out b/tests/test3.out
index 1610013c..a0c33cec 100644
--- a/tests/test3.out
+++ b/tests/test3.out
@@ -8,3 +8,4 @@ Test 7 pass
 Test 8 pass
 Test 9 pass
 Test 10 pass
+Test 11 pass