Commits

Anonymous committed 1cfb59c

Better ResourceObserver

  • Participants
  • Parent commits 0142bfb

Comments (0)

Files changed (14)

docs/dev/issues.txt

 * Finding available refactorings
 
 
+Better Resource Observing
+=========================
+
+I propose to have separate methods for handling different file
+operations::
+
+  class ResourceObserver(object):
+
+      def resource_changed(self, resource):
+          pass
+
+      def resource_moved(self, source, destination):
+          pass
+
+      def resource_removed(self, resource):
+          pass
+
+      def resource_created(self, resource):
+          pass
+
+
+In one extreme we have to separate every change.  Some observers are
+interested in details and some are not.  Reducing the number of
+messages requires more checks on both interested and uninterested
+observers.
+
+The other problem that we might face is that messages might overlap
+each other.  For example when we move a resource a new resource would
+be created, too;  Should we call `resource_created`?
+
+
 Memory Management
 =================
 
 information and testing.
 
 
-Task `Progress`
-===============
-
-Currently refactorings cannot be stopped.  I propose to add an object
-for stopping and showing the progress of a time consuming task::
-
-  class TaskHandle(object):
-
-      def stop(self):
-          pass
-
-      def is_stopped(self):
-          pass
-
-      def create_job_set(self, name=None, total=None):
-          pass
-
-      def get_job_sets(self):
-          pass
-
-      def add_observer(self):
-          pass
-
-  class JobSet(object):
-
-      def start_job(self, name):
-          if self.current_job is not None:
-              self.finished_job()
-          self.set_job_name(name)
-
-      def finished_job(self):
-          """Raises `TaskInterruptedError` if this task was interrupted"""
-
-      def get_percent_done(self):
-          pass
-
-
-It requires adding an argument to all refactorings and other time
-consuming tasks and changing them to use this object.
-
-
 Better Concluded Data
 =====================
 

docs/dev/workingon.txt

-Stoppable Refactorings
-======================
+Small Stories
+=============
 
-- Adding `rope.refactor.taskhandle` module
-- Stopping time consuming tasks
+- Renaming `ResourceObserver.resource_removed()` to `resource_moved`
+- Reporting resource_created in validate_resource; use mtime
+- Changing parents after creations
+- Multiple calls to validate
+- Reporting resource_created in resource_moved
 
-  - find occurrences
-  - move
-  - inline
-  - change signature
-  - introduce factory
-
-- Document task handles in `rope.refactor`
+* Caching `Project.get_all_files()`
+* `Project.get_files()` does not care about ignored resources
+* Caching `PyCore.get_python_files()`?
+* Caching `PyCore.get_source_folders()`?
 
 * Codeassist on function header with pydocs!
-* Caching `Project.get_all_files()`, `PyCore.get_source_folders()`
 
 * Validating callinfo
 * Codeassist only shows saved data

rope/base/change.py

                         self.project._get_resource_path(destination))
         new_resource = self.project.get_resource(destination)
         for observer in list(self.project.observers):
-            observer.resource_removed(resource, new_resource)
+            observer.resource_moved(resource, new_resource)
 
     def create(self, resource):
         if resource.is_folder():
         else:
             self._create_file(resource.path)
         for observer in list(self.project.observers):
-            observer.resource_changed(resource)
+            observer.resource_created(resource)
 
     def remove(self, resource):
         fscommands = self._get_fscommands(resource)

rope/base/evaluate.py

 import compiler
 
-import rope.base.exceptions
+import rope.base.pynames
 import rope.base.pyobjects
-import rope.base.pynames
+from rope.base import exceptions
 
 
 class StatementEvaluator(object):
         if pyname.get_object() != rope.base.pyobjects.get_unknown():
             try:
                 self.result = pyname.get_object().get_attribute(node.attrname)
-            except rope.base.exceptions.AttributeNotFoundError:
+            except exceptions.AttributeNotFoundError:
                 self.result = None
 
     def visitCallFunc(self, node):

rope/base/oi/objectinfo.py

 
     def _init_validation(self):
         self.objectdb.validate_files()
-        observer = rope.base.project.ResourceObserver(self._resource_changed,
-                                                      self._resource_removed)
+        observer = rope.base.project.ResourceObserver(
+            changed=self._resource_changed, moved=self._resource_moved,
+            removed=self._resource_moved)
         files = []
         for path in self.objectdb.get_files():
             resource = self.to_pyobject.file_to_resource(path)
         except SyntaxError:
             pass
 
-    def _resource_removed(self, resource, new_resource=None):
+    def _resource_moved(self, resource, new_resource=None):
         self.observer.remove_resource(resource)
         if new_resource is not None:
             self.objectdb.file_moved(resource.real_path, new_resource.real_path)

rope/base/oi/staticoi.py

 def _analyze_node(pycore, pydefined, should_analyze):
     if should_analyze is not None and not should_analyze(pydefined):
         return
-#    if hasattr(pydefined, 'get_name'):
-#        print pydefined.get_name()
+    #    if hasattr(pydefined, 'get_name'):
+    #        print pydefined.get_name()
     visitor = SOIVisitor(pycore, pydefined, should_analyze)
     for child in pydefined.get_ast().getChildNodes():
         compiler.walk(child, visitor)

rope/base/oi/transform.py

                 self.transform(pyobject.values))
 
     def Tuple_to_textual(self, pyobject):
-        objects = [self.transform(holding) for holding in pyobject.get_holding_objects()]
+        objects = [self.transform(holding)
+                   for holding in pyobject.get_holding_objects()]
         return tuple(['builtin', 'tuple'] + objects)
 
     def Set_to_textual(self, pyobject):

rope/base/project.py

 
     """
 
-    def __init__(self, changed, removed):
+    def __init__(self, changed=None, moved=None, created=None, removed=None):
         self.changed = changed
+        self.moved = moved
+        self.created = created
         self.removed = removed
 
     def resource_changed(self, resource):
         """It is called when the resource changes"""
-        self.changed(resource)
+        if self.changed is not None:
+            self.changed(resource)
 
-    def resource_removed(self, resource, new_resource=None):
-        """It is called when a resource no longer exists
+    def resource_moved(self, resource, new_resource):
+        """It is called when a resource is moved"""
+        if self.removed is not None:
+            self.removed(resource, new_resource)
 
-        `new_resource` is the destination if we know it, otherwise it
-        is None.
+    def resource_created(self, resource):
+        """Is called when a new resource is created"""
+        if self.created is not None:
+            self.created(resource)
 
-        """
-        self.removed(resource, new_resource)
+    def resource_removed(self, resource):
+        """Is called when a new resource is removed"""
+        if self.removed is not None:
+            self.removed(resource)
 
     def validate(self, resource):
         """Validate the existence of this resource and its children.
 
     def add_resource(self, resource):
         """Add a resource to the list of interesting resources"""
-        self.resources[resource] = self.timekeeper.getmtime(resource)
+        if resource.exists():
+            self.resources[resource] = self.timekeeper.getmtime(resource)
+        else:
+            self.resources[resource] = None
 
     def remove_resource(self, resource):
         """Add a resource to the list of interesting resources"""
         if self._is_parent_changed(changed):
             changes.add_changed(changed.parent)
 
-    def _update_changes_caused_by_removed(self, changes, resource,
-                                          new_resource=None):
+    def _update_changes_caused_by_moved(self, changes, resource,
+                                        new_resource=None):
         if resource in self.resources:
             changes.add_removed(resource, new_resource)
+        if new_resource in self.resources:
+            changes.add_created(new_resource)
         if resource.is_folder():
             for file in list(self.resources):
                 if resource.contains(file):
-                    new_file = self._calculate_new_resource(resource,
-                                                            new_resource, file)
+                    new_file = self._calculate_new_resource(
+                        resource, new_resource, file)
                     changes.add_removed(file, new_file)
         if self._is_parent_changed(resource):
             changes.add_changed(resource.parent)
     def _is_parent_changed(self, child):
         return child.parent in self.resources
 
-    def resource_removed(self, resource, new_resource=None):
+    def resource_moved(self, resource, new_resource):
         changes = _Changes()
-        self._update_changes_caused_by_removed(changes, resource, new_resource)
+        self._update_changes_caused_by_moved(changes, resource, new_resource)
         self._perform_changes(changes)
 
+    def resource_created(self, resource):
+        changes = _Changes()
+        self._update_changes_caused_by_created(changes, resource)
+        self._perform_changes(changes)
+
+    def _update_changes_caused_by_created(self, changes, resource):
+        if resource in self.resources:
+            changes.add_created(resource)
+        if self._is_parent_changed(resource):
+            changes.add_changed(resource.parent)
+
+    def resource_removed(self, resource):
+        changes = _Changes()
+        self._update_changes_caused_by_moved(changes, resource)
+        self._perform_changes(changes)
+
+    def _perform_changes(self, changes):
+        for resource in changes.changes:
+            self.observer.resource_changed(resource)
+            self.resources[resource] = self.timekeeper.getmtime(resource)
+        for resource, new_resource in changes.moves.iteritems():
+            self.resources[resource] = None
+            if new_resource is not None:
+                self.observer.resource_moved(resource, new_resource)
+            else:
+                self.observer.resource_removed(resource)
+        for resource in changes.creations:
+            self.observer.resource_created(resource)
+            self.resources[resource] = self.timekeeper.getmtime(resource)
+
     def validate(self, resource):
         moved = self._search_resource_moves(resource)
         changed = self._search_resource_changes(resource)
         changes = _Changes()
         for file in moved:
             if file in self.resources:
-                self._update_changes_caused_by_removed(changes, file)
+                self._update_changes_caused_by_moved(changes, file)
         for file in changed:
             if file in self.resources:
                 self._update_changes_caused_by_changed(changes, file)
+        for file in self._search_resource_creations(resource):
+            if file in self.resources:
+                changes.add_created(file)
         self._perform_changes(changes)
 
-    def _perform_changes(self, changes):
-        for resource in changes.changes:
-            self.observer.resource_changed(resource)
-            self.resources[resource] = self.timekeeper.getmtime(resource)
-        for resource, new_resource in changes.moves.iteritems():
-            self.observer.resource_removed(resource, new_resource)
+    def _search_resource_creations(self, resource):
+        creations = set()
+        if resource in self.resources and resource.exists() and \
+           self.resources[resource] == None:
+            creations.add(resource)
+        if resource.is_folder():
+            for file in self.resources:
+                if file.exists() and resource.contains(file) and \
+                   self.resources[file] == None:
+                    creations.add(file)
+        return creations
 
     def _search_resource_moves(self, resource):
         all_moved = set()
         return changed
 
     def _is_changed(self, resource):
+        if self.resources[resource] is None:
+            return False
         return self.resources[resource] != self.timekeeper.getmtime(resource)
 
     def _calculate_new_resource(self, main, new_main, resource):
 
     def __init__(self):
         self.changes = set()
+        self.creations = set()
         self.moves = {}
 
     def add_changed(self, resource):
     def add_removed(self, resource, new_resource=None):
         self.moves[resource] = new_resource
 
+    def add_created(self, resource):
+        self.creations.add(resource)
 
 class _IgnoredResources(object):
 

rope/base/pycore.py

         self.project = project
         self.module_map = {}
         self.classes = None
+        callback = self._invalidate_resource_cache
         observer = rope.base.project.ResourceObserver(
-            self._invalidate_resource_cache, self._invalidate_resource_cache)
+            changed=callback, moved=callback, removed=callback)
         self.observer = rope.base.project.FilteredResourceObserver(observer)
         self.project.add_observer(self.observer)
         self.object_info = rope.base.oi.objectinfo.ObjectInfoManager(project)
             self._init_automatic_soi()
 
     def _init_automatic_soi(self):
-        observer = rope.base.project.ResourceObserver(self._file_changed,
-                                                      self._file_changed)
+        callback = self._file_changed
+        observer = rope.base.project.ResourceObserver(
+            changed=callback, moved=callback, removed=callback)
         self.project.add_observer(observer)
 
     def _file_changed(self, resource, new_resource=None):

rope/ui/actionhelpers.py

 
         thread = threading.Thread(target=calculate)
         thread.start()
-        toplevel.focus_set()
         toplevel.grab_set()
+        stop_button.focus_set()
         toplevel.mainloop()
         toplevel.destroy()
         if calculate.interrupted:
         toplevel.bind('<Escape>', lambda event: ok())
         toplevel.bind('<Return>', lambda event: ok())
         ok_button.grid(row=1)
-        toplevel.focus_set()
+        toplevel.grab_set()
+        ok_button.focus_set()
 
     def _editor_changed(self):
         active_editor = self.editor_manager.active_editor

rope/ui/fileactions.py

                  MenuAddress(['File', 'Edit Project config.py'], None, 4),
                  ['all', 'none']))
 actions.append(SimpleAction('validate_project', validate_project, 'C-x p v',
-                            MenuAddress(['File', 'Validate Project Files'], 'v', 4)))
+                            MenuAddress(['File', 'Validate/Refresh Project Files'], 'v', 4)))
 actions.append(SimpleAction('sync_project', sync_project, 'C-x p s',
                             MenuAddress(['File', 'Sync Project To Disk'], None, 4)))
 

rope/ui/fileeditor.py

         #    self.editor.getWidget()['state'] = Tkinter.DISABLED
 
     def _register_observers(self):
+        callback = self._file_was_modified
         self.observer = FilteredResourceObserver(
-            ResourceObserver(self._file_was_modified, self._file_was_removed),
+            ResourceObserver(changed=callback, moved=callback, removed=callback),
             [self.file])
         if self.project is not None:
             self.project.add_observer(self.observer)

ropetest/projecttest.py

                                                            [sample_file]))
         sample_file.remove()
         self.assertEquals(1, sample_observer.change_count)
-        self.assertEquals((sample_file, None), sample_observer.last_moved)
+        self.assertEquals(sample_file, sample_observer.last_removed)
 
     def test_resource_change_observer2(self):
         sample_file = self.project.root.create_file('my_file.txt')
                                                            [my_file]))
         os.remove(my_file.real_path)
         self.project.validate(root)
-        self.assertEquals((my_file, None), sample_observer.last_moved)
+        self.assertEquals(my_file, sample_observer.last_removed)
         self.assertEquals(1, sample_observer.change_count)
 
     def test_revalidating_files_and_no_changes2(self):
                                                            [my_folder]))
         testutils.remove_recursively(my_folder.real_path)
         self.project.validate(root)
-        self.assertEquals((my_folder, None), sample_observer.last_moved)
+        self.assertEquals(my_folder, sample_observer.last_removed)
         self.assertEquals(1, sample_observer.change_count)
 
     def test_removing_and_adding_resources_to_filtered_observer(self):
         folder2 = self.project.root.create_folder('folder2')
         self.assertFalse(folder1.contains(folder2))
 
+    def test_validating_when_created(self):
+        root = self.project.root
+        my_file = self.project.get_file('my_file.txt')
+        sample_observer = _SampleObserver()
+        self.project.add_observer(
+            FilteredResourceObserver(sample_observer, [my_file]))
+        file(my_file.real_path, 'w').close()
+        self.project.validate(root)
+        self.assertEquals(my_file, sample_observer.last_created)
+        self.assertEquals(1, sample_observer.change_count)
+
+    def test_validating_twice_when_created(self):
+        root = self.project.root
+        my_file = self.project.get_file('my_file.txt')
+        sample_observer = _SampleObserver()
+        self.project.add_observer(
+            FilteredResourceObserver(sample_observer, [my_file]))
+        file(my_file.real_path, 'w').close()
+        self.project.validate(root)
+        self.project.validate(root)
+        self.assertEquals(my_file, sample_observer.last_created)
+        self.assertEquals(1, sample_observer.change_count)
+
+    def test_changes_and_adding_resources(self):
+        root = self.project.root
+        file1 = self.project.get_file('file1.txt')
+        file2 = self.project.get_file('file2.txt')
+        file1.create()
+        sample_observer = _SampleObserver()
+        self.project.add_observer(
+            FilteredResourceObserver(sample_observer, [file1, file2]))
+        file1.move(file2.path)
+        self.assertEquals(2, sample_observer.change_count)
+        self.assertEquals(file2, sample_observer.last_created)
+        self.assertEquals((file1, file2), sample_observer.last_moved)
+
 
 class _MockTimeKeepter(object):
 
         self.change_count = 0
         self.last_changed = None
         self.last_moved = None
+        self.last_created = None
+        self.last_removed = None
 
     def resource_changed(self, resource):
         self.last_changed = resource
         self.change_count += 1
 
-    def resource_removed(self, resource, new_resource=None):
+    def resource_moved(self, resource, new_resource):
         self.last_moved = (resource, new_resource)
         self.change_count += 1
 
+    def resource_created(self, resource):
+        self.last_created = resource
+        self.change_count += 1
+
+    def resource_removed(self, resource):
+        self.last_removed = resource
+        self.change_count += 1
+
 
 class OutOfProjectTest(unittest.TestCase):