From 7eb6fdddafb7ee98afd8dc170a23bfbb32708994 Mon Sep 17 00:00:00 2001
From: Steve Bennett <steveb@workware.net.au>
Date: Fri, 24 Feb 2023 12:59:56 +1000
Subject: dict with: return script result

Previously dict with returned the new dict value.
Also fix an issue in the case where a dict element
mirrors the name of the dictionary.

Fixes: #241

Signed-off-by: Steve Bennett <steveb@workware.net.au>
---
 jim.c            | 18 +++++++++++++-----
 jim.h            |  1 +
 tests/dict.test  | 12 ++++++++++++
 tests/dict2.test |  9 +++++++--
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/jim.c b/jim.c
index ac78fda..772435e 100644
--- a/jim.c
+++ b/jim.c
@@ -7863,6 +7863,8 @@ int Jim_DictKeysVector(Jim_Interp *interp, Jim_Obj *dictPtr,
  *
  * If flags & JIM_ERRMSG, then failure to remove the key is considered an error
  * and JIM_ERR is returned. Otherwise it is ignored and JIM_OK is returned.
+ *
+ * Normally the result is stored in the interp result. If JIM_NORESULT is set, this is not done.
  */
 int Jim_SetDictKeysVector(Jim_Interp *interp, Jim_Obj *varNamePtr,
     Jim_Obj *const *keyv, int keyc, Jim_Obj *newObjPtr, int flags)
@@ -7932,7 +7934,10 @@ int Jim_SetDictKeysVector(Jim_Interp *interp, Jim_Obj *varNamePtr,
     if (Jim_SetVariable(interp, varNamePtr, varObjPtr) != JIM_OK) {
         goto err;
     }
-    Jim_SetResult(interp, varObjPtr);
+
+    if (!(flags & JIM_NORESULT)) {
+        Jim_SetResult(interp, varObjPtr);
+    }
     return JIM_OK;
   err:
     if (shared) {
@@ -15270,10 +15275,13 @@ static int JimDictWith(Jim_Interp *interp, Jim_Obj *dictVarName, Jim_Obj *const
             }
 
             for (i = 0; i < len; i += 2) {
-                /* This will be NULL if the variable no longer exists, thus deleting the variable */
-                objPtr = Jim_GetVariable(interp, dictValues[i], 0);
-                newkeyv[keyc] = dictValues[i];
-                Jim_SetDictKeysVector(interp, dictVarName, newkeyv, keyc + 1, objPtr, 0);
+                /* If the an element mirrors the dictionary name, skip it to avoid creating a recursive data structure */
+                if (Jim_StringCompareObj(interp, dictVarName, dictValues[i], 0) != 0) {
+                    /* This will be NULL if the variable no longer exists, thus deleting the variable */
+                    objPtr = Jim_GetVariable(interp, dictValues[i], 0);
+                    newkeyv[keyc] = dictValues[i];
+                    Jim_SetDictKeysVector(interp, dictVarName, newkeyv, keyc + 1, objPtr, JIM_NORESULT);
+                }
             }
             Jim_Free(newkeyv);
         }
diff --git a/jim.h b/jim.h
index 4162e5e..55788d6 100644
--- a/jim.h
+++ b/jim.h
@@ -154,6 +154,7 @@ extern "C" {
 #define JIM_ENUM_ABBREV 2       /* Jim_GetEnum() - Allow unambiguous abbreviation */
 #define JIM_UNSHARED 4          /* Jim_GetVariable() - return unshared object */
 #define JIM_MUSTEXIST 8         /* Jim_SetDictKeysVector() - fail if non-existent */
+#define JIM_NORESULT 16         /* Jim_SetDictKeysVector() - don't store the result in the interp result */
 
 /* Flags for Jim_SubstObj() */
 #define JIM_SUBST_NOVAR 1 /* don't perform variables substitutions */
diff --git a/tests/dict.test b/tests/dict.test
index f77d95e..0c2c395 100644
--- a/tests/dict.test
+++ b/tests/dict.test
@@ -114,6 +114,18 @@ test dict-5.2 {Dict with} {
 	}
 	dictsort [a]
 } {a B}
+test dict-5.3 {Dict with return value} {
+	proc a {} {
+		set x [dict create a b c d]
+		dict with x {
+			set a B
+			unset c
+			# dict with should return the script return value
+			set retvalue ok
+		}
+	}
+	a
+} {ok}
 
 test dict-22.1 {dict with command} {
     list [catch {dict with} msg] $msg
diff --git a/tests/dict2.test b/tests/dict2.test
index 99cf605..573d17a 100644
--- a/tests/dict2.test
+++ b/tests/dict2.test
@@ -1272,8 +1272,13 @@ test dict-23.4 {dict with usage} -body {
 } -returnCodes error -result {wrong # args: should be "dict with dictVar ?key ...? script"}
 
 test dict-23.5 {dict with badvar} -constraints jim -body {
-	dict with dictnulls {lsort [info locals]}
-} -returnCodes ok -result [list ab\0c de\0f \0ghi kl\0m]
+    proc a {dict} {
+	dict with dict {
+	    lsort [info locals]
+	}
+    }
+    a $dictnulls
+} -returnCodes ok -result [list \0ghi ab\0c dict]
 
 test dict-23.6 {dict with baddict} -body {
 	dict with dictbad {}
-- 
cgit v1.1