dict: Fix [dict values] with duplicate values

The script implementation of dict values was not correctly handling the case
where a dictionary had duplicate values.

Signed-off-by: Steve Bennett <steveb@workware.net.au>

5 files changed, 38 insertions, 48 deletions

diff --git a/jim-array.c b/jim-array.c
index cd3e784..4213bc3 100644
--- a/jim-array.c
+++ b/jim-array.c
@@ -79,8 +79,7 @@ static int array_cmd_get(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
         }
     }
 
-    /* Return a list of keys and values where the keys match the pattern */
-    return Jim_DictValues(interp, objPtr, patternObj);
+    return Jim_DictMatchTypes(interp, objPtr, patternObj, JIM_DICTMATCH_KEYS, JIM_DICTMATCH_KEYS | JIM_DICTMATCH_VALUES);
 }
 
 static int array_cmd_names(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
@@ -91,7 +90,7 @@ static int array_cmd_names(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
         return JIM_OK;
     }
 
-    return Jim_DictKeys(interp, objPtr, argc == 1 ? NULL : argv[1]);
+    return Jim_DictMatchTypes(interp, objPtr, argc == 1 ? NULL : argv[1], JIM_DICTMATCH_KEYS, JIM_DICTMATCH_KEYS);
 }
 
 static int array_cmd_unset(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
diff --git a/jim.c b/jim.c
index 443da61..f35865f 100644
--- a/jim.c
+++ b/jim.c
@@ -14270,55 +14270,43 @@ static int Jim_RenameCoreCommand(Jim_Interp *interp, int argc, Jim_Obj *const *a
     return Jim_RenameCommand(interp, Jim_String(argv[1]), Jim_String(argv[2]));
 }
 
-#define JIM_DICTMATCH_VALUES 0x0001
-
-typedef void JimDictMatchCallbackType(Jim_Interp *interp, Jim_Obj *listObjPtr, Jim_HashEntry *he, int type);
-
-static void JimDictMatchKeys(Jim_Interp *interp, Jim_Obj *listObjPtr, Jim_HashEntry *he, int type)
-{
-    Jim_ListAppendElement(interp, listObjPtr, (Jim_Obj *)he->key);
-    if (type & JIM_DICTMATCH_VALUES) {
-        Jim_ListAppendElement(interp, listObjPtr, Jim_GetHashEntryVal(he));
-    }
-}
+#define JIM_DICTMATCH_KEYS 0x0001
+#define JIM_DICTMATCH_VALUES 0x002
 
 /**
- * Like JimHashtablePatternMatch, but for dictionaries.
+ * match_type must be one of JIM_DICTMATCH_KEYS or JIM_DICTMATCH_VALUES
+ * return_types should be either or both
  */
-static Jim_Obj *JimDictPatternMatch(Jim_Interp *interp, Jim_HashTable *ht, Jim_Obj *patternObjPtr,
-    JimDictMatchCallbackType *callback, int type)
+int Jim_DictMatchTypes(Jim_Interp *interp, Jim_Obj *objPtr, Jim_Obj *patternObj, int match_type, int return_types)
 {
     Jim_HashEntry *he;
-    Jim_Obj *listObjPtr = Jim_NewListObj(interp, NULL, 0);
-
-    /* Check for the non-pattern case. We can do this much more efficiently. */
+    Jim_Obj *listObjPtr;
     Jim_HashTableIterator htiter;
-    JimInitHashTableIterator(ht, &htiter);
-    while ((he = Jim_NextHashEntry(&htiter)) != NULL) {
-        if (patternObjPtr == NULL || JimGlobMatch(Jim_String(patternObjPtr), Jim_String((Jim_Obj *)he->key), 0)) {
-            callback(interp, listObjPtr, he, type);
-        }
-    }
-
-    return listObjPtr;
-}
 
-
-int Jim_DictKeys(Jim_Interp *interp, Jim_Obj *objPtr, Jim_Obj *patternObjPtr)
-{
     if (SetDictFromAny(interp, objPtr) != JIM_OK) {
         return JIM_ERR;
     }
-    Jim_SetResult(interp, JimDictPatternMatch(interp, objPtr->internalRep.ptr, patternObjPtr, JimDictMatchKeys, 0));
-    return JIM_OK;
-}
 
-int Jim_DictValues(Jim_Interp *interp, Jim_Obj *objPtr, Jim_Obj *patternObjPtr)
-{
-    if (SetDictFromAny(interp, objPtr) != JIM_OK) {
-        return JIM_ERR;
+    listObjPtr = Jim_NewListObj(interp, NULL, 0);
+
+    JimInitHashTableIterator(objPtr->internalRep.ptr, &htiter);
+    while ((he = Jim_NextHashEntry(&htiter)) != NULL) {
+        if (patternObj) {
+            Jim_Obj *matchObj = (match_type == JIM_DICTMATCH_KEYS) ? (Jim_Obj *)he->key : Jim_GetHashEntryVal(he);
+            if (!JimGlobMatch(Jim_String(patternObj), Jim_String(matchObj), 0)) {
+                /* no match */
+                continue;
+            }
+        }
+        if (return_types & JIM_DICTMATCH_KEYS) {
+            Jim_ListAppendElement(interp, listObjPtr, (Jim_Obj *)he->key);
+        }
+        if (return_types & JIM_DICTMATCH_VALUES) {
+            Jim_ListAppendElement(interp, listObjPtr, Jim_GetHashEntryVal(he));
+        }
     }
-    Jim_SetResult(interp, JimDictPatternMatch(interp, objPtr->internalRep.ptr, patternObjPtr, JimDictMatchKeys, JIM_DICTMATCH_VALUES));
+
+    Jim_SetResult(interp, listObjPtr);
     return JIM_OK;
 }
 
@@ -14479,6 +14467,7 @@ static int JimDictWith(Jim_Interp *interp, Jim_Obj *dictVarName, Jim_Obj *const
 static int Jim_DictCoreCommand(Jim_Interp *interp, int argc, Jim_Obj *const *argv)
 {
     Jim_Obj *objPtr;
+    int types = JIM_DICTMATCH_KEYS;
     int option;
     static const char * const options[] = {
         "create", "get", "set", "unset", "exists", "keys", "size", "info",
@@ -14545,12 +14534,15 @@ static int Jim_DictCoreCommand(Jim_Interp *interp, int argc, Jim_Obj *const *arg
             }
             return JIM_OK;
 
+        case OPT_VALUES:
+            types = JIM_DICTMATCH_VALUES;
+            /* fallthru */
         case OPT_KEYS:
             if (argc != 3 && argc != 4) {
                 Jim_WrongNumArgs(interp, 2, argv, "dictionary ?pattern?");
                 return JIM_ERR;
             }
-            return Jim_DictKeys(interp, argv[2], argc == 4 ? argv[3] : NULL);
+            return Jim_DictMatchTypes(interp, argv[2], argc == 4 ? argv[3] : NULL, types, types);
 
         case OPT_SIZE:
             if (argc != 3) {
diff --git a/jim.h b/jim.h
index 4719292..58e6e56 100644
--- a/jim.h
+++ b/jim.h
@@ -801,8 +801,11 @@ JIM_EXPORT int Jim_DictPairs(Jim_Interp *interp,
         Jim_Obj *dictPtr, Jim_Obj ***objPtrPtr, int *len);
 JIM_EXPORT int Jim_DictAddElement(Jim_Interp *interp, Jim_Obj *objPtr,
         Jim_Obj *keyObjPtr, Jim_Obj *valueObjPtr);
-JIM_EXPORT int Jim_DictKeys(Jim_Interp *interp, Jim_Obj *objPtr, Jim_Obj *patternObj);
-JIM_EXPORT int Jim_DictValues(Jim_Interp *interp, Jim_Obj *dictObjPtr, Jim_Obj *patternObjPtr);
+
+#define JIM_DICTMATCH_KEYS 0x0001
+#define JIM_DICTMATCH_VALUES 0x002
+
+JIM_EXPORT int Jim_DictMatchTypes(Jim_Interp *interp, Jim_Obj *objPtr, Jim_Obj *patternObj, int match_type, int return_types);
 JIM_EXPORT int Jim_DictSize(Jim_Interp *interp, Jim_Obj *objPtr);
 JIM_EXPORT int Jim_DictInfo(Jim_Interp *interp, Jim_Obj *objPtr);
 JIM_EXPORT Jim_Obj *Jim_DictMerge(Jim_Interp *interp, int objc, Jim_Obj *const *objv);
diff --git a/stdlib.tcl b/stdlib.tcl
index aef6983..9cffb93 100644
--- a/stdlib.tcl
+++ b/stdlib.tcl
@@ -154,11 +154,6 @@ proc {dict remove} {dictionary {args key}} {
 	return $dictionary
 }
 
-# Script-based implementation of 'dict values'
-proc {dict values} {dictionary {pattern *}} {
-	dict keys [lreverse $dictionary] $pattern
-}
-
 # Script-based implementation of 'dict for'
 proc {dict for} {vars dictionary script} {
 	if {[llength $vars] != 2} {
diff --git a/tests/dict2.test b/tests/dict2.test
index 91e88a6..54d4d0d 100644
--- a/tests/dict2.test
+++ b/tests/dict2.test
@@ -197,6 +197,7 @@ test dict-7.9 {dict values command} -returnCodes error -body {
 test dict-7.10 {dict values command} -returnCodes error -body {
     dict values a
 } -result {missing value to go with key}
+test dict-7.11 {dict values with duplicate values} {dict values {a b c b} b} {b b}
 
 test dict-8.1 {dict size command} {dict size {}} 0
 test dict-8.2 {dict size command} {dict size {a b}} 1