jamesfredley commented on code in PR #16732:
URL: https://github.com/apache/grails-core/pull/16732#discussion_r4368470646


##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -199,30 +246,160 @@ class NavigableMap implements Map<String, Object>, 
Cloneable {
                 mergeMaps(rootMap, newPath , subMap, (Map) sourceValue, 
parseFlatKeys)
                 newValue = subMap
             } else {
-                newValue = sourceValue
+                newValue = mergedValue(sourceValue)
             }
             if (isNestedSet && newValue == null) {
-                if (path) {
-
-                    def subMap = rootMap.get(path)
-                    if (subMap instanceof Map) {
-                        subMap.remove(sourceKey)
-                    }
-                    def keysToRemove = rootMap.keySet().findAll() { String key 
->
-                        key.startsWith("${path}.")
-                    }
-                    for (key in keysToRemove) {
-                        rootMap.remove(key)
-                    }
-                }
+                // the flat key of the removed property, those below it and 
its indexed aliases go with it;
+                // the flat keys of its siblings stay
+                String removedKey = path ? "${path}.${sourceKey}".toString() : 
sourceKey
+                removeKeysBelow(rootMap, removedKey, ['.', '['])
+                rootMap.remove(removedKey)
                 targetMap.remove(sourceKey)
             } else {
+                String flatKey = path ? "${path}.${sourceKey}".toString() : 
sourceKey
                 if (path) {
-                    rootMap.put("${path}.${sourceKey}".toString(), newValue)
+                    mergeMapEntry(rootMap, flatKey, newValue)
                 }
                 mergeMapEntry(targetMap, sourceKey, newValue)
+                if (replacedBy(currentValue, newValue)) {
+                    // the element keys of the container the value replaces go 
with it; the flat keys of the
+                    // entries of a block stay, as they always have
+                    removeKeysBelow(rootMap, flatKey, ['['])
+                }
+                writeElements(rootMap, flatKey, newValue)
+            }
+        }
+    }
+
+    /**
+     * Writes the element keys of a list, {@code a.rows[0]} and, for an 
element that is a map, {@code a.rows[0].name},
+     * as the indexed keys of a configuration file write them, so that a list 
is read alike however it was written.
+     * A map that is not a block of this configuration, an element of a list 
or a value an indexed key wrote, has
+     * its entries written the same way; a block writes its own flat keys as 
it is merged.
+     */
+    private void writeElements(NavigableMap rootMap, String flatKey, Object 
value) {
+        if (value instanceof List) {
+            int index = 0
+            for (Object element in (List) value) {

Review Comment:
   `map.app.names = ['one']` then `map.app.names[0] = 'two'` leaves the 
generated `app.names[0]` alias as `'one'`. `toProperties()` then reports both 
values, and `other.merge(map, true)` replays the stale alias and puts `'one'` 
back.
   
   Do not let a snapshot alias override the current contents of a mutable list. 
Please cover that assignment and the merge that follows.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to