[core] node rename : assert the node cannot be renamed in lock mode (+ other fixes suggested by copilot)

Author: Alxiice

meshroom/core/graph.py

@@ -519,19 +519,22 @@ def renameNode(self, node: Node, newName: str):
 
         Args:
             node (Node): Node to rename.
-            newName (str): New name of the node. Must be unique in the graph.
+            newName (str): New name of the node.
         """
         # Handle empty string
         if not newName:
             return
+        if node.getLocked():
+            logging.warning(f"Cannot rename node {node} because of the locked status")
+            return
         usedNames = {n._name for n in self._nodes if n != node}
         # Make sure we rename to an available name
         if newName in usedNames:
             newName = self._createUniqueNodeName(newName, usedNames)
         # Rename in the dict model
         self._nodes.rename(node._name, newName)
         # Finally rename the node name property and notify Qt
-        node._name = newName   
+        node._name = newName
         node.nodeNameChanged.emit()
 
     def copyNode(self, srcNode: Node, withEdges: bool=False):

meshroom/core/node.py

@@ -913,7 +913,7 @@ def nameToLabel(self, name):
         Returns:
             str: the high-level label from the technical node name
         """
-        t, idx = name.split("_") if "_" in name else (name, "1")
+        t, idx = name.rsplit("_", 1) if "_" in name else (name, "1")
         return f"{t}{idx if int(idx) > 1 else ''}"
 
     def getDocumentation(self):

meshroom/ui/graph.py

@@ -939,13 +939,11 @@ def addNewNode(self, nodeType, position=None, **kwargs):
     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.
+        In this function the last `_N` index is removed, then all special characters
+        (everything except letters and numbers) are removed. 
+        The name uniqueness will be ensured later by adding a suffix (e.g. `_1`, `_2`, ...)
         
-        As we remove all special characters, only one underscore is allowed in the node name. 
-        Label can be used if multiple underscores are needed.
+        Labels can be used to have special characters in the displayed name.
 
         Args:
             node (Node): Node to rename.

meshroom/ui/qml/GraphEditor/Node.qml

@@ -90,7 +90,7 @@ Item {
             root.y = root.node.y
         }
         function onNameChanged() {
-            // Make sure when the node name changes the node label is updated
+            // HACK: Make sure when the node name changes the node label is updated
             root.nodeLabel = ""
             // Restore binding to root.node.label
             root.nodeLabel = Qt.binding(function() { return root.node.label; })

meshroom/ui/qml/GraphEditor/NodeEditor.qml

@@ -34,7 +34,7 @@ Panel {
             // Set the display node type only if it is not contained in the node name
             const nodeType = _reconstruction.selectedNode.nodeType
             root.displayNodeType = nodeName.startsWith(nodeType + "_") ? "" : nodeType
-        } 
+        }
     }
 
     Connections {
@@ -116,6 +116,7 @@ Panel {
                 id: nodeNameField
                 visible: root.node !== null
                 text: root.displayNodeName
+                // For some reason the validator doesn't always work
                 validator: RegularExpressionValidator { regularExpression: /^[0-9A-Za-z]+$/ }
                 font.bold: true
                 readOnly: true

tests/test_graph.py

@@ -296,16 +296,21 @@ def test_rename_nodes():
     ls0 = graph.addNewNode("Ls")
     ls1 = graph.addNewNode("Ls")
     ls2 = graph.addNewNode("Ls")
-    
+
     # Test with empty string
     assert ls0.name == "Ls_1"
     graph.renameNode(ls0, "")
     assert ls0.name == "Ls_1"
-    
+
     # Rename
     graph.renameNode(ls0, "nodels")
     assert ls0.name == "nodels"
     graph.renameNode(ls1, "nodels")
     assert ls1.name == "nodels_1"
     graph.renameNode(ls2, "nodels")
     assert ls2.name == "nodels_2"
+    
+    # Check we cannot rename in locked mode
+    ls0.setLocked(True)
+    graph.renameNode(ls0, "lockedLs")
+    assert ls0.name == "nodels"