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

This was introduced in 7b3377e78c16b3ba74edb141ca0dd2ec895806b9

1 files changed, 14 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);
+        }
+    }
 }