IP: Support editing Root URLs and Groups after creation

Needs:
* Detect any cycle created in an edited Source property of a Group
* Test new Unicode icon for Edit button on Windows (and Linux)
* Automated tests
* Release notes
* Link to issue #98 in commit message

While testing, don't forget to check:
* Fix titles to update properly when name changes
  (ex: for direct children of ResourceGroupNode)
* Focus on Name field by default when open Edit dialog
* Menuitem & its accelerator

src/crystal/browser/__init__.py

@@ -31,6 +31,7 @@
 from crystal.util.xthreading import (
     bg_call_later, fg_affinity, fg_call_later, fg_call_and_wait, set_is_quitting
 )
+from functools import partial
 import os
 import time
 from typing import ContextManager, Iterator, Optional
@@ -195,17 +196,25 @@ def _create_actions(self) -> None:
         self._new_root_url_action = Action(wx.ID_ANY,
             'New &Root URL...',
             wx.AcceleratorEntry(wx.ACCEL_CTRL, ord('R')),
-            self._on_add_url,
+            self._on_new_root_url,
             enabled=(not self._readonly),
             button_bitmap=TREE_NODE_ICONS()['entitytree_root_resource'],
             button_label='New &Root URL...')
         self._new_group_action = Action(wx.ID_ANY,
             'New &Group...',
             wx.AcceleratorEntry(wx.ACCEL_CTRL, ord('G')),
-            self._on_add_group,
+            self._on_new_group,
             enabled=(not self._readonly),
             button_bitmap=dict(DEFAULT_FOLDER_ICON_SET())[wx.TreeItemIcon_Normal],
             button_label='New &Group...')
+        self._edit_action = Action(wx.ID_ANY,
+            '&Edit...',
+            accel=wx.AcceleratorEntry(wx.ACCEL_NORMAL, wx.WXK_RETURN),
+            action_func=self._on_edit_entity,
+            enabled=False,
+            # FIXME: Verify that ✏️ displays okay on Windows 8+
+            # FIXME: Verify that '' is the correct value for Windows
+            button_label=decorate_label('✏️', '&Edit...', ''))
         self._forget_action = Action(wx.ID_ANY,
             '&Forget',
             wx.AcceleratorEntry(wx.ACCEL_CTRL, wx.WXK_BACK),
@@ -221,11 +230,13 @@ def _create_actions(self) -> None:
         self._update_members_action = Action(wx.ID_ANY,
             'Update &Members',
             accel=None,
-            action_func=self._on_update_group_membership,
+            action_func=self._on_update_group_members,
             enabled=False,
             button_label=decorate_label('🔎', 'Update &Members', ' '))
         self._view_action = Action(wx.ID_ANY,
             '&View',
+            # TODO: Consider adding Space as a alternate accelerator
+            #wx.AcceleratorEntry(wx.ACCEL_NORMAL, wx.WXK_SPACE),
             wx.AcceleratorEntry(wx.ACCEL_CTRL|wx.ACCEL_SHIFT, ord('O')),
             self._on_view_entity,
             enabled=False,
@@ -271,6 +282,7 @@ def _create_menu_bar(self, raw_frame: wx.Frame) -> wx.MenuBar:
         entity_menu = wx.Menu()
         self._new_root_url_action.append_menuitem_to(entity_menu)
         self._new_group_action.append_menuitem_to(entity_menu)
+        self._edit_action.append_menuitem_to(entity_menu)
         self._forget_action.append_menuitem_to(entity_menu)
         entity_menu.AppendSeparator()
         self._download_action.append_menuitem_to(entity_menu)
@@ -329,25 +341,24 @@ def _create_entity_tree(self, parent: wx.Window, progress_listener: OpenProjectP
     def _create_button_bar(self, parent: wx.Window):
         readonly = self._readonly  # cache
 
-        add_url_button = self._new_root_url_action.create_button(parent, name='cr-add-url-button')
-
-        add_group_button = self._new_group_action.create_button(parent, name='cr-add-group-button')
-
-        remove_entity_button = self._forget_action.create_button(parent, name='cr-forget-button')
-
+        new_root_url_button = self._new_root_url_action.create_button(parent, name='cr-add-url-button')
+        new_group_button = self._new_group_action.create_button(parent, name='cr-add-group-button')
+        edit_entity_button = self._edit_action.create_button(parent, name='cr-edit-button')
+        forget_entity_button = self._forget_action.create_button(parent, name='cr-forget-button')
         download_button = self._download_action.create_button(parent, name='cr-download-button')
-
         update_members_button = self._update_members_action.create_button(
             parent, name='cr-update-members-button')
 
         view_button = self._view_action.create_button(parent, name='cr-view-button')
 
         content_sizer = wx.BoxSizer(wx.HORIZONTAL)
-        content_sizer.Add(add_url_button)
+        content_sizer.Add(new_root_url_button)
+        content_sizer.AddSpacer(_WINDOW_INNER_PADDING)
+        content_sizer.Add(new_group_button)
         content_sizer.AddSpacer(_WINDOW_INNER_PADDING)
-        content_sizer.Add(add_group_button)
+        content_sizer.Add(edit_entity_button)
         content_sizer.AddSpacer(_WINDOW_INNER_PADDING)
-        content_sizer.Add(remove_entity_button)
+        content_sizer.Add(forget_entity_button)
         content_sizer.AddSpacer(_WINDOW_INNER_PADDING * 2)
         content_sizer.AddStretchSpacer()
         content_sizer.Add(download_button)
@@ -432,54 +443,106 @@ def _on_close_frame(self, event: wx.CloseEvent) -> None:
 
     # === Entity Pane: Events ===
 
-    def _on_add_url(self, event: wx.CommandEvent) -> None:
-        def root_url_exists(url: str) -> bool:
-            r = self.project.get_resource(url)
-            if r is None:
-                return False
-            rr = self.project.get_root_resource(r)
-            return rr is not None
-
+    def _on_new_root_url(self, event: wx.CommandEvent) -> None:
         NewRootUrlDialog(
-            self._frame, self._on_add_url_dialog_ok,
-            url_exists_func=root_url_exists,
+            self._frame, self._on_new_root_url_dialog_ok,
+            url_exists_func=self._root_url_exists,
             initial_url=self._suggested_url_or_url_pattern_for_selection or '',
             initial_name=self._suggested_name_for_selection or '',
         )
 
+    def _root_url_exists(self, url: str) -> bool:
+        r = self.project.get_resource(url)
+        if r is None:
+            return False
+        rr = self.project.get_root_resource(r)
+        return rr is not None
+
     @fg_affinity
-    def _on_add_url_dialog_ok(self, name: str, url: str) -> None:
+    def _on_new_root_url_dialog_ok(self, name: str, url: str) -> None:
         if url == '':
             raise ValueError('Invalid blank URL')
         try:
             RootResource(self.project, name, Resource(self.project, url))
         except RootResource.AlreadyExists:
             raise ValueError('Invalid duplicate URL')
 
-    def _on_add_group(self, event: wx.CommandEvent) -> None:
+    @fg_affinity
+    def _on_edit_root_url_dialog_ok(self, rr: RootResource, name: str, url: str) -> None:
+        if url != rr.url:
+            raise ValueError()
+        rr.name = name
+
+        # TODO: This update should happen in response to an event
+        #       fired by the entity itself.
+        self.entity_tree.root.update_title_of_descendants()  # update names in titles
+
+    def _on_new_group(self, event: wx.CommandEvent) -> None:
         try:
             NewGroupDialog(
-                self._frame, self._on_add_group_dialog_ok,
+                self._frame, self._on_new_group_dialog_ok,
                 self.project,
                 initial_url_pattern=self._suggested_url_or_url_pattern_for_selection or '',
                 initial_source=self._suggested_source_for_selection,
                 initial_name=self._suggested_name_for_selection or '')
         except CancelLoadUrls:
             pass
 
-    def _on_add_group_dialog_ok(self, name: str, url_pattern: str, source):
+    @fg_affinity
+    def _on_new_group_dialog_ok(self, name: str, url_pattern: str, source: ResourceGroupSource) -> None:
         # TODO: Validate user input:
-        #       * Is name or url_pattern empty?
-        #       * Is name or url_pattern already taken?
-        rg = ResourceGroup(self.project, name, url_pattern, source)
+        #       * Is url_pattern empty?
+        #       * Is url_pattern already taken?
+        ResourceGroup(self.project, name, url_pattern, source)
+
+    @fg_affinity
+    def _on_edit_group_dialog_ok(self, rg: ResourceGroup, name: str, url_pattern: str, source: ResourceGroupSource) -> None:
+        if url_pattern != rg.url_pattern:
+            raise ValueError()
+        (rg.name, rg.source) = (name, source)
+
+        # TODO: This update should happen in response to an event
+        #       fired by the entity itself.
+        self.entity_tree.root.update_title_of_descendants()  # update names in titles
 
-    def _on_forget_entity(self, event):
+    def _on_edit_entity(self, event) -> None:
         selected_entity_pair = self.entity_tree.selected_entity_pair
         selected_or_related_entity = selected_entity_pair[0] or selected_entity_pair[1]
+        assert selected_or_related_entity is not None
+
+        if isinstance(selected_or_related_entity, RootResource):
+            rr = selected_or_related_entity
+            NewRootUrlDialog(
+                self._frame, partial(self._on_edit_root_url_dialog_ok, rr),
+                url_exists_func=self._root_url_exists,
+                initial_url=rr.url,
+                initial_name=rr.name,
+                is_edit=True,
+            )
+        elif isinstance(selected_or_related_entity, ResourceGroup):
+            rg = selected_or_related_entity
+            try:
+                NewGroupDialog(
+                    self._frame, partial(self._on_edit_group_dialog_ok, rg),
+                    self.project,
+                    initial_url_pattern=rg.url_pattern,
+                    initial_source=rg.source,
+                    initial_name=rg.name,
+                    is_edit=True)
+            except CancelLoadUrls:
+                pass
+        else:
+            raise AssertionError()
+
+    def _on_forget_entity(self, event) -> None:
+        selected_entity_pair = self.entity_tree.selected_entity_pair
+        selected_or_related_entity = selected_entity_pair[0] or selected_entity_pair[1]
+        assert selected_or_related_entity is not None
 
         selected_or_related_entity.delete()
-        # TODO: This update() should happen in response to a delete
-        #       event fired by the entity itself.
+
+        # TODO: This update() should happen in response to an event
+        #       fired by the entity itself.
         self.entity_tree.update()
 
     def _on_download_entity(self, event) -> None:
@@ -545,7 +608,7 @@ def fg_task() -> None:
                 fg_call_and_wait(fg_task)
         bg_call_later(bg_task)
 
-    def _on_update_group_membership(self, event):
+    def _on_update_group_members(self, event):
         selected_entity = self.entity_tree.selected_entity
         selected_entity.update_members()
 
@@ -611,6 +674,9 @@ def _on_selected_entity_changed(self, event: wx.TreeEvent) -> None:
         selected_or_related_entity = selected_entity_pair[0] or selected_entity_pair[1]
 
         readonly = self._readonly  # cache
+        self._edit_action.enabled = (
+            (not readonly) and
+            isinstance(selected_or_related_entity, (ResourceGroup, RootResource)))
         self._forget_action.enabled = (
             (not readonly) and
             isinstance(selected_or_related_entity, (ResourceGroup, RootResource)))

src/crystal/browser/entitytree.py

@@ -832,8 +832,10 @@ def entity(self) -> RootResource:
     # === Comparison ===
 
     def __eq__(self, other):
-        return isinstance(other, RootResourceNode) and (
-            self.root_resource == other.root_resource)
+        return (
+            isinstance(other, RootResourceNode) and
+            self.root_resource == other.root_resource
+        )
     def __hash__(self):
         return hash(self.root_resource)
 
@@ -1090,8 +1092,10 @@ def on_more_expanded(self, more_node: MorePlaceholderNode) -> None:
     # === Comparison ===
 
     def __eq__(self, other) -> bool:
-        return isinstance(other, ResourceGroupNode) and (
-            self.resource_group == other.resource_group)
+        return (
+            isinstance(other, ResourceGroupNode) and
+            self.resource_group == other.resource_group
+        )
     def __hash__(self) -> int:
         return hash(self.resource_group)
 

src/crystal/browser/new_group.py

@@ -5,7 +5,7 @@
 from crystal.progress import CancelLoadUrls
 from crystal.util.unicode_labels import decorate_label
 from crystal.util.wx_bind import bind
-from crystal.util.wx_dialog import position_dialog_initially
+from crystal.util.wx_dialog import CreateButtonSizer, position_dialog_initially
 from crystal.util.wx_static_box_sizer import wrap_static_box_sizer_child
 from crystal.util.xos import is_linux
 import sys
@@ -31,6 +31,7 @@ def __init__(self,
             initial_url_pattern: str='',
             initial_source: ResourceGroupSource=None,
             initial_name: str='',
+            is_edit: bool=False,
             ) -> None:
         """
         Arguments:
@@ -54,7 +55,8 @@ def __init__(self,
             raise
 
         dialog = self.dialog = wx.Dialog(
-            parent, title='New Group',
+            parent,
+            title=('New Group' if not is_edit else 'Edit Group'),
             name='cr-new-group-dialog',
             style=wx.DEFAULT_DIALOG_STYLE|wx.RESIZE_BORDER)
         dialog_sizer = wx.BoxSizer(wx.VERTICAL)
@@ -96,17 +98,22 @@ def __init__(self,
 
         content_sizer = wx.BoxSizer(wx.VERTICAL)
         content_sizer.Add(
-            self._create_fields(dialog, initial_url_pattern, initial_source, initial_name),
+            self._create_fields(dialog, initial_url_pattern, initial_source, initial_name, is_edit),
             flag=wx.EXPAND)
         content_sizer.Add(preview_box, proportion=1, flag=wx.EXPAND|preview_box_flags, border=preview_box_border)
 
+        ok_button_id = (wx.ID_NEW if not is_edit else wx.ID_SAVE)
         dialog_sizer.Add(content_sizer, proportion=1, flag=wx.EXPAND|wx.ALL,
             border=_WINDOW_INNER_PADDING)
-        dialog_sizer.Add(dialog.CreateButtonSizer(wx.OK|wx.CANCEL), flag=wx.EXPAND|wx.BOTTOM,
+        dialog_sizer.Add(CreateButtonSizer(dialog, ok_button_id, wx.ID_CANCEL), flag=wx.EXPAND|wx.BOTTOM,
             border=_WINDOW_INNER_PADDING)
 
         if not fields_hide_hint_when_focused():
-            self.pattern_field.SetFocus()  # initialize focus
+            # Initialize focus
+            if not is_edit:
+                self.pattern_field.SetFocus()
+            else:
+                self.name_field.SetFocus()
         self._update_preview_urls()
 
         position_dialog_initially(dialog)
@@ -121,6 +128,7 @@ def _create_fields(self,
             initial_url_pattern: str,
             initial_source: ResourceGroupSource,
             initial_name: str,
+            is_edit: bool,
             ) -> wx.Sizer:
         fields_sizer = wx.FlexGridSizer(cols=2,
             vgap=_FORM_ROW_SPACING, hgap=_FORM_LABEL_INPUT_SPACING)
@@ -134,6 +142,7 @@ def _create_fields(self,
         bind(self.pattern_field, wx.EVT_TEXT, self._on_pattern_field_changed)
         self.pattern_field.Hint = 'https://example.com/post/*'
         self.pattern_field.SetSelection(-1, -1)  # select all upon focus
+        self.pattern_field.Enabled = not is_edit
         pattern_field_sizer.Add(self.pattern_field, flag=wx.EXPAND)
         pattern_field_sizer.Add(wx.StaticText(parent, label='# = digit, @ = alpha, * = any nonslash, ** = any'), flag=wx.EXPAND)
 
@@ -227,7 +236,7 @@ def _on_pattern_field_changed(self, event) -> None:
 
     def _on_button(self, event: wx.CommandEvent) -> None:
         btn_id = event.GetEventObject().GetId()
-        if btn_id == wx.ID_OK:
+        if btn_id in (wx.ID_NEW, wx.ID_SAVE):
             self._on_ok(event)
         elif btn_id == wx.ID_CANCEL:
             self._on_cancel(event)