Fix #14 (#142)

* Enforce a unique name for each task in each task tree when adding a new task.

* Ignore TaskInterface objects in the task tree

* Renaming: Enforce a unique name for each task in the task tree

* Renaming: Override exit_rename instead of modifying it.

* Fix parameter naming inconsistency in create_task. Update the documentation.

* Renaming: Check name uniqueness when adding a task from the tree widget.

* measurement_edition: Prefix the name of tasks that are copy-pasted.

* Add new tests and update old ones

* Address reviews

- Replace the root attribute in the task configuration window by a
future_parent attribute which contains the future parent of the task
being created.

- Add visual feedback when the name is not valid (this information is
also stored in the config object)

- Readd the check_parameters call in the BaseConfig constructor.

* Removed unused import

* tasks: run checks only when manager is known

* Update loop_config.py

* Add comment and delete whitespace

exopy/measurement/workspace/measurement_edition.enaml

@@ -26,7 +26,7 @@ from ...utils.widgets.qt_clipboard import CLIPBOARD
 from ...utils.widgets.qt_tree_widget import QtTreeWidget
 from ...utils.widgets.tree_nodes import TreeNode
 from ...utils.widgets.qt_tree_menu import \
-    (CutAction, PasteAction, NewAction, RenameAction, DeleteAction)
+    (CutAction, NewAction, RenameAction, DeleteAction)
 from ...tasks.api import (BaseTask, SimpleTask, ComplexTask)
 from ...tasks.utils.building import build_task_from_config
 from ..editors.base_editor import BaseEditor
@@ -90,14 +90,36 @@ enamldef TaskCopyAction(Action):
         CLIPBOARD.instance = copy
 
 
-def build_task(workbench):
+enamldef TaskPasteAction(Action):
+    """ Pastes the task currently in the paste buffer into the current node.
+
+    """
+    #: Context holding the informations about on which task this action was
+    #: invoked. See exopy.utils.widgets.tree_node for a more complete
+    #: description.
+    attr action_context
+
+    text = 'Paste'
+    visible << action_context['pasteable']
+    triggered::
+        widget, node, obj, nid = action_context['data']
+        new_data = CLIPBOARD.instance
+        used_names = obj.root.get_used_names()
+        for i in new_data.traverse():
+            if hasattr(i, 'name') and i.name in used_names:
+                i.name = "(copy)" + i.name
+        widget._append(node, obj, new_data, False)
+
+
+def build_task(workbench, future_parent):
     """Call the Command responsible for building a task.
 
     """
     core = workbench.get_plugin('enaml.workbench.core')
     ui = workbench.get_plugin('enaml.workbench.ui')
     return core.invoke_command('exopy.tasks.create_task',
-                               {'parent_ui': ui.window})
+                               {'parent_ui': ui.window,
+                                'future_parent': future_parent})
 
 
 enamldef SimpleMenu(Menu): menu:
@@ -114,12 +136,12 @@ enamldef SimpleMenu(Menu): menu:
         action_context << context
         factory = build_task
         mode = 'Add before'
-        kwargs = {'workbench': workspace.workbench}
+        kwargs = {'workbench': workspace.workbench, 'future_parent': context['data'][2]}
     NewAction:
         action_context << context
         factory = build_task
         mode = 'Add after'
-        kwargs = {'workbench': workspace.workbench}
+        kwargs = {'workbench': workspace.workbench, 'future_parent': context['data'][2]}
 
     Action:
         separator = True
@@ -129,7 +151,7 @@ enamldef SimpleMenu(Menu): menu:
     TaskCopyAction:
         workspace = menu.workspace
         action_context << context
-    PasteAction:
+    TaskPasteAction:
         action_context << context
 
     Action:
@@ -158,17 +180,17 @@ enamldef ComplexMenu(Menu): menu:
     NewAction:
         action_context << context
         factory = build_task
-        kwargs = {'workbench': workspace.workbench}
+        kwargs = {'workbench': workspace.workbench, 'future_parent': context['data'][2]}
     NewAction:
         action_context << context
         factory = build_task
         mode = 'Add before'
-        kwargs = {'workbench': workspace.workbench}
+        kwargs = {'workbench': workspace.workbench, 'future_parent': context['data'][2]}
     NewAction:
         action_context << context
         factory = build_task
         mode = 'Add after'
-        kwargs = {'workbench': workspace.workbench}
+        kwargs = {'workbench': workspace.workbench, 'future_parent': context['data'][2]}
 
     Action:
         separator = True
@@ -185,7 +207,7 @@ enamldef ComplexMenu(Menu): menu:
     TaskCopyAction:
         workspace = menu.workspace
         action_context << context
-    PasteAction:
+    TaskPasteAction:
         action_context << context
 
     Action:
@@ -210,6 +232,10 @@ enamldef TaskTreeNode(TreeNode):
         return obj.name + '(' + obj.task_id + ')'
     enter_rename => (obj):
         return obj.name
+    exit_rename => (obj, label):
+        forbidden_names = obj.root.get_used_names()
+        if label not in forbidden_names:
+            obj.name = label
 
 
 class _MeasEditionModel(Atom):

exopy/tasks/configs/base_config_views.enaml

@@ -13,6 +13,7 @@ from enaml.core.api import Conditional
 from enaml.layout.api import hbox, vbox, spacer
 from enaml.widgets.api import (Container, MultilineField, Field, GroupBox,
                                Label)
+from enaml.colors import parse_color
 
 
 enamldef BaseConfigView(Container):
@@ -34,6 +35,7 @@ enamldef BaseConfigView(Container):
     Field: t_f:
         text := config.task_name
         submit_triggers = ['lost_focus', 'return_pressed', 'auto_sync']
+        background << parse_color("#FFFFFF") if config.name_valid else parse_color("#FF000030")
 
 
 enamldef PyConfigView(BaseConfigView): main:

exopy/tasks/configs/base_configs.py

@@ -11,7 +11,7 @@
 """
 import random
 
-from atom.api import (Atom, Bool, Unicode, Subclass, ForwardTyped)
+from atom.api import (Atom, Bool, Unicode, Subclass, ForwardTyped, Typed)
 
 from inspect import getdoc
 
@@ -33,6 +33,9 @@ class BaseTaskConfig(Atom):
     """Base class for task configurer.
 
     """
+    #: Future parent in the task hierarchy used to enforce name uniqueness.
+    future_parent = Typed(BaseTask)
+
     #: Task manager, necessary to retrieve task implementations.
     manager = ForwardTyped(task_manager)
 
@@ -45,17 +48,28 @@ class BaseTaskConfig(Atom):
     #: Bool indicating if the build can be done.
     ready = Bool(False)
 
+    #: Bool indicating if the name is valid.
+    name_valid = Bool(False)
+
     def __init__(self, **kwargs):
         super(BaseTaskConfig, self).__init__(**kwargs)
         # Force check to ensure that the possible default value of task_name
         # is tested.
         self.check_parameters()
 
     def check_parameters(self):
-        """The only parameter required is a valid task name.
+        """The only parameter required is a unique task name.
 
         """
-        self.ready = bool(self.task_name)
+        # Checking the parameters make sense only if the manager is known
+        if not self.manager:
+            return
+
+        names = []
+        if self.future_parent:
+            names = self.future_parent.root.get_used_names()
+        self.name_valid = self.task_name != "" and self.task_name not in names
+        self.ready = self.name_valid
 
     def build_task(self):
         """This method use the user parameters to build the task object
@@ -69,6 +83,13 @@ def build_task(self):
         """
         raise NotImplementedError()
 
+    def _post_setattr_future_parent(self, _old, _new):
+        """If the object was not initialized with a future_parent, we weren't
+        able to perform all the checks so we perform them now
+
+        """
+        self.check_parameters()
+
     def _post_setattr_task_name(self, old, new):
         """Everytime the task name change check whether ornot it is valid.
 
@@ -78,7 +99,8 @@ def _post_setattr_task_name(self, old, new):
     def _default_task_name(self):
         names = self.manager.auto_task_names
         if names:
-            return random.choice(names)
+            name = random.choice(names)
+            return name
         else:
             return ''
 

exopy/tasks/configs/loop_config.py

@@ -35,7 +35,15 @@ def check_parameters(self):
         """Ensure that both this config and the subconfig parameters are valid.
 
         """
-        if self.task_name != '':
+        # Run the checks only if the manager is known.
+        if not self.manager:
+            return
+
+        names = []
+        if self.future_parent:
+            names = self.future_parent.root.get_used_names()
+        self.name_valid = self.task_name != '' and self.task_name not in names
+        if self.name_valid:
             if self.use_subtask:
                 if self.subconfig is not None:
                     self.subconfig.task_name = self.task_name

exopy/tasks/manifest.enaml

@@ -269,6 +269,9 @@ Parameters:
 parent_ui : optional
     Optional parent widget for the dialog.
 
+future_parent : BaseTask
+    Future parent task of the new task will be inserted.
+
 Returns:
 -------
 task : BaseTask

exopy/tasks/tasks/base_tasks.py

@@ -1229,6 +1229,21 @@ def build_from_config(cls, config, dependencies):
         task.register_preferences()
         return task
 
+    def get_used_names(self):
+        """Return the list of all names used in the tree
+        Returns
+        -------
+        names : List(str)
+            List of all the names used in the tree.
+
+        """
+        names = []
+        for i in self.traverse():
+            # Effectively ignores TaskInterface objects
+            if hasattr(i, 'name'):
+                names.append(i.name)
+        return names
+
     # =========================================================================
     # --- Private API ---------------------------------------------------------
     # =========================================================================

exopy/tasks/tasks/base_views.enaml

@@ -88,13 +88,16 @@ enamldef RootTaskView(BaseTaskView): main:
         BaseTaskView.refresh(self)
         editor.refresh()
 
-    # TODO pass the future parent task so that config can do name inspections
-    # and avoid duplicate names.
-    func create_new_task():
+    func create_new_task(future_parent):
         """Create a new task to insert into the hierarchy.
 
+        Parameters
+        ----------
+        future_parent:
+            Future parent of the task
+
         """
-        return core.invoke_command('exopy.tasks.create_task', dict(widget=self))
+        return core.invoke_command('exopy.tasks.create_task', dict(parent_ui=self, future_parent=future_parent))
 
     func get_interfaces_for(obj):
         """Get the available interfaces for a given object.

exopy/tasks/tasks/task_editor.enaml

@@ -60,7 +60,7 @@ enamldef EmptyEditorButton(PushButton):
     text = 'Add a first task'
 
     clicked ::
-        task = editor.root.create_new_task()
+        task = editor.root.create_new_task(editor.task)
         if task:
             editor.task.add_child_task(0, task)
 
@@ -245,7 +245,7 @@ class TaskEditor(Container):
             """Handler for the add entry of the menu.
 
             """
-            task = self.root.create_new_task()
+            task = self.root.create_new_task(self.task)
             if task:
                 if position == 'after':
                     index += 1

exopy/tasks/utils/building.py

@@ -26,9 +26,12 @@ def create_task(event):
 
     Parameters
     ----------
-    widget : optional
+    parent_ui : optional
         Optional parent widget for the dialog.
 
+    future_parent : BaseTask
+        Future parent of the task
+
     Returns:
     -------
     task : BaseTask
@@ -37,7 +40,8 @@ def create_task(event):
     """
     manager = event.workbench.get_plugin('exopy.tasks')
     dialog = BuilderView(manager=manager,
-                         parent=event.parameters.get('widget'))
+                         parent=event.parameters.get('parent_ui'),
+                         future_parent=event.parameters.get('future_parent'))
     result = dialog.exec_()
     if result:
         return dialog.config.build_task()

exopy/tasks/widgets/building.enaml

@@ -38,6 +38,9 @@ enamldef BuilderView(Dialog): dial:
     #: Reference to the task manager.
     alias manager : selector.manager
 
+    #: Future parent of the task
+    attr future_parent
+
     #: Config object corresponding to the currently selected task.
     attr config
 
@@ -52,6 +55,7 @@ enamldef BuilderView(Dialog): dial:
             if config is None:
                 dial.config = None
                 return [Label(text='No config found for this task.')]
+            config.future_parent = future_parent
             dial.config = config
             return [view]
         else:

exopy/utils/widgets/qt_tree_widget.py

@@ -783,8 +783,7 @@ def _on_nid_changed(self, nid, col):
         if new_label != old_label:
             if new_label != '':
                 node.exit_rename(obj, new_label)
-            else:
-                self._set_label(nid, old_label)
+            self._set_label(nid, node.get_label(obj))
 
     def _children_replaced(self, change):
         """ Handles the children of a node being completely replaced.