[core/ui] node rename : Add documentation + improve node rename & fix small ui issue

Author: Alxiice

meshroom/common/core.py

@@ -51,7 +51,18 @@ def add(self, obj):
         assert key not in self._objects
         self._objects[key] = obj
 
-    def rename(self, oldKey, newKey):
+    def rename(self, oldKey: str, newKey: str):
+        """ Rename an element in the dict model
+
+        Args:
+            oldKey (str): Previous key name of the element to replace.
+            newKey (str): New key name to insert in the model.
+
+        Raises:
+            KeyError: if the new name is already used.
+        """
+        if newKey in self._objects.keys():
+            raise KeyError(f"Key {newKey} is already in use in {self}")
         obj = self._objects[oldKey]
         self._objects[newKey] = obj
         del self._objects[oldKey]

meshroom/common/qt.py

@@ -167,7 +167,18 @@ def replace(self, i, obj):
         self._objects[i] = obj
         self.dataChanged.emit(self.index(i), self.index(i), [])
 
-    def rename(self, oldKey, newKey):
+    def rename(self, oldKey: str, newKey: str):
+        """ Rename an element in the model
+
+        Args:
+            oldKey (str): Previous key name of the element to replace.
+            newKey (str): New key name to insert in the model.
+
+        Raises:
+            KeyError: if the new name is already used.
+        """
+        if newKey in self._objectByKey.keys():
+            raise KeyError(f"Key {newKey} is already in use in {self}")
         obj = self._objectByKey[oldKey]
         index = self.indexOf(obj)
         self._objectByKey[newKey] = obj

meshroom/core/graph.py

@@ -514,6 +514,12 @@ def addNode(self, node, uniqueName=None):
         return node
 
     def renameNode(self, node: Node, newName: str):
+        """ Rename a node in the Node Graph
+
+        Args:
+            node (Node): Node to rename.
+            newName (str): New name of the node. Must be unique in the graph.
+        """
         self._nodes.rename(node._name, newName)
         # Finally rename
         node._name = newName   

meshroom/ui/commands.py

@@ -153,15 +153,20 @@ def undoImpl(self):
 
 class RenameNodeCommand(GraphCommand):
     def __init__(self, graph, node, name, parent=None):
+        """ Command to rename a node. The new name should not be used yet.
+        """
         super().__init__(graph, parent)
         self.node = node
         self.oldName = node._name
         self.name = name
 
     def redoImpl(self):
-        newName = self.graph._createUniqueNodeName(self.name, [n._name for n in self.graph._nodes])
+        newName = self.name
+        usedNames = {n._name for n in self.graph._nodes}
+        if newName in usedNames:
+            newName = self.graph._createUniqueNodeName(self.name, usedNames)
         self.setText(f"Rename Node {self.oldName} to {newName}")
-        self.graph.renameNode(self.node, newName)
+        self.graph.renameNode(self.node, self.name)
         return newName
 
     def undoImpl(self):

meshroom/ui/graph.py

@@ -936,13 +936,32 @@ def addNewNode(self, nodeType, position=None, **kwargs):
         return self.push(commands.AddNodeCommand(self._graph, nodeType, position=position, **kwargs))
 
     @Slot(Node, str, result=str)
-    def renameNode(self, node, newName):
+    def renameNode(self, node: Node, newName: str):
+        """ Triggers the node renaming.
+        
+        The name we send here doesn't need to be unique here because we will ensure this later in 
+        the RenameNodeCommand. In this function the last `_N` index is removed, then all special characters
+        (everything except letters and numbers) are removed. Then a `_N` index will be added to
+        ensure the node name unicity.
+        
+        As we remove all special characters, only one underscore is allowed in the node name. 
+        Label can be used if multiple underscores are needed.
+
+        Args:
+            node (Node): Node to rename.
+            newName (str): New name to set.
+
+        Returns:
+            str: The final name of the node.
+        """
         newName = "_".join(newName.split("_")[:-1]) if "_" in newName else newName
         # Eliminate all characters except digits and letters
         newName = re.sub(r"[^0-9a-zA-Z]", "", newName)
-        if not newName:
+        # Create unique name
+        uniqueName = self._graph._createUniqueNodeName(newName, {n._name for n in self._graph._nodes if n != node})
+        if not newName or uniqueName == node._name:
             return ""
-        return self.push(commands.RenameNodeCommand(self._graph, node, newName))
+        return self.push(commands.RenameNodeCommand(self._graph, node, uniqueName))
 
     def moveNode(self, node: Node, position: Position):
         """

meshroom/ui/qml/GraphEditor/NodeEditor.qml

@@ -85,7 +85,7 @@ Panel {
     function validateNodeNameChange(name) {
         if (root.node && name.trim() !== "") {
             const newNodeName = _reconstruction.renameNode(_reconstruction.selectedNode, name.trim())
-            if (newNodeName == "") {
+            if (newNodeName === "") {
                 root.displayNodeName = root.nodeName
                 root.validatedNodeName = root.nodeName
             } else {
@@ -95,6 +95,7 @@ Panel {
         }
     }
     function cancelNodeNameChange() {
+        // HACK: Set to an empty string to force the text to be set to the previous value.
         root.displayNodeName = ""
         root.displayNodeName = root.validatedNodeName
     }
@@ -201,7 +202,7 @@ Panel {
             // Show node type if the node name does not start with "nodeType_"
             Label {
                 text: "(" + root.displayNodeType + ")"
-                visible: root.displayNodeType !== ""
+                visible: root.displayNodeType !== "" && _reconstruction.selectedNode
                 topPadding: 4
                 bottomPadding: 4
             }