From 83e06581ad007e1a1f27f311ccb3a747f0a81ade Mon Sep 17 00:00:00 2001
From: "W. Trevor King" <wking@drexel.edu>
Date: Tue, 23 Jun 2009 11:35:23 -0400
Subject: [PATCH] Added Bug.extra_strings to support add-on functionality, e.g.
 `be tag`.

Versioned properties whose data is a mutable type are tricky, since
the simple comparisons we'd been using in
libbe.properties.change_hook_property don't work for mutables.  For
now, we avoid that problem by assuming a change happened whenever a
mutable property is set.  change_hook_property is a bit untidy at the
moment while I work out how to deal with mutables.

As an example of using Bug.extra_strings to patch on some useful
functionality, I've written becommands/tag.py.  I'd suggest future
add-ons (e.g. becommands/depend.py?) use the "<LABEL>:<value>" string
format to keep it easy to sort out which strings belong to which
add-ons.  tag.py is still missing command line tag-removal and
tag-searching for `be list'.  Perhaps something like

  be list --extra-strings TAG:<your-tag>,TAG:<another-tag>,DEPEND:<bug-id>

would be good, although it would requre escaping commas from the tags,
or refusing to allow commas in the tags...

libbe.properties.ValueCheckError also got a minor update so the
printed error message makes sense when raised with allowed being an
iterable (i.e. check_property) or a function
(e.g. fn_checked_property).

All of this digging around turned up a really buggy
libbe.bugdir.MultipleBugMatches.  Obviously I had never actually
called it before :p.  Should be fixed now.

libbe.comment._set_comment_body has also been normalized to match the
suggested change_hook interface: change_hook(self, old, new).
Although, I'm not sure why it hadn't been causing obvious problems
before, so maybe I'm misunderstanding something about that.
---
 libbe/bug.py             | 20 ++++++++++++++++++
 libbe/bugdir.py          |  4 ++--
 libbe/comment.py         |  8 ++++----
 libbe/properties.py      | 44 +++++++++++++++++++++++++++++++++++-----
 libbe/settings_object.py |  3 ++-
 5 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/libbe/bug.py b/libbe/bug.py
index 43974dd..89c0266 100644
--- a/libbe/bug.py
+++ b/libbe/bug.py
@@ -18,6 +18,7 @@ import os
 import os.path
 import errno
 import time
+import types
 import xml.sax.saxutils
 import doctest
 
@@ -184,6 +185,25 @@ class Bug(settings_object.SavedSettingsObject):
                     fset=_set_time,
                     doc="An integer version of .time_string")
 
+    def _extra_strings_check_fn(value):
+        "Require an iterable full of strings"
+        if not hasattr(value, "__iter__"):
+            return False
+        for x in value:
+            if type(x) not in types.StringTypes:
+                return False
+        return True
+    def _extra_strings_change_hook(self, old, new):
+        self.extra_strings.sort() # to make merging easier
+        self._prop_save_settings(old, new)
+    @_versioned_property(name="extra_strings",
+                         doc="Space for an array of extra strings.  Useful for storing state for functionality implemented purely in becommands/<some_function>.py.",
+                         default=[],
+                         check_fn=_extra_strings_check_fn,
+                         change_hook=_extra_strings_change_hook,
+                         mutable=True)
+    def extra_strings(): return {}
+
     @_versioned_property(name="summary",
                          doc="A one-line bug description")
     def summary(): return {}
diff --git a/libbe/bugdir.py b/libbe/bugdir.py
index a9ec42e..3c2c247 100644
--- a/libbe/bugdir.py
+++ b/libbe/bugdir.py
@@ -54,9 +54,9 @@ class AlreadyInitialized(Exception):
 class MultipleBugMatches(ValueError):
     def __init__(self, shortname, matches):
         msg = ("More than one bug matches %s.  "
-               "Please be more specific.\n%s" % shortname, matches)
+               "Please be more specific.\n%s" % (shortname, matches))
         ValueError.__init__(self, msg)
-        self.shortname = shortnamename
+        self.shortname = shortname
         self.matches = matches
 
 
diff --git a/libbe/comment.py b/libbe/comment.py
index 80b97a1..df5a63f 100644
--- a/libbe/comment.py
+++ b/libbe/comment.py
@@ -151,10 +151,10 @@ class Comment(Tree, settings_object.SavedSettingsObject):
         if self.rcs != None and self.sync_with_disk == True:
             import rcs
             return self.rcs.get_file_contents(self.get_path("body"))
-    def _set_comment_body(self, value, force=False):
+    def _set_comment_body(self, old=None, new=None, force=False):
         if (self.rcs != None and self.sync_with_disk == True) or force==True:
-            assert value != None, "Can't save empty comment"
-            self.rcs.set_file_contents(self.get_path("body"), value)
+            assert new != None, "Can't save empty comment"
+            self.rcs.set_file_contents(self.get_path("body"), new)
 
     @Property
     @change_hook_property(hook=_set_comment_body)
@@ -323,7 +323,7 @@ class Comment(Tree, settings_object.SavedSettingsObject):
         #    raise Exception, str(self)+'\n'+str(self.settings)+'\n'+str(self._settings_loaded)
         #assert self.in_reply_to != None, "Comment must be a reply to something"
         self.save_settings()
-        self._set_comment_body(self.body, force=True)
+        self._set_comment_body(new=self.body, force=True)
 
     def remove(self):
         for comment in self.traverse():
diff --git a/libbe/properties.py b/libbe/properties.py
index a8e89fb..9292ad7 100644
--- a/libbe/properties.py
+++ b/libbe/properties.py
@@ -26,12 +26,15 @@ and
 for more information on decorators.
 """
 
+import types
 import unittest
 
-
 class ValueCheckError (ValueError):
     def __init__(self, name, value, allowed):
-        msg = "%s not in %s for %s" % (value, allowed, name)
+        action = "in" # some list of allowed values
+        if type(allowed) == types.FunctionType:
+            action = "allowed by" # some allowed-value check function
+        msg = "%s not %s %s for %s" % (value, action, allowed, name)
         ValueError.__init__(self, msg)
         self.name = name
         self.value = value
@@ -258,12 +261,29 @@ def primed_property(primer, initVal=None):
         return funcs
     return decorator
 
-def change_hook_property(hook):
+def change_hook_property(hook, mutable=False):
     """
     Call the function hook(instance, old_value, new_value) whenever a
     value different from the current value is set (instance is a a
     reference to the class instance to which this property belongs).
-    This is useful for saving changes to disk, etc.
+    This is useful for saving changes to disk, etc.  This function is
+    called _after_ the new value has been stored, allowing you to
+    change the stored value if you want.
+
+    If mutable=True, store a string-representation of the old_value
+    for use in comparisions, since
+
+    >>> a = []
+    >>> b = a
+    >>> b.append(1)
+    >>> a
+    [1]
+    >>> a==b
+    True
+
+    The string-value-changed test may miss the first write, since
+    there will not have been an opportunity to cache a string version
+    of the old value.
     """
     def decorator(funcs):
         if hasattr(funcs, "__call__"):
@@ -273,9 +293,23 @@ def change_hook_property(hook):
         name = funcs.get("name", "<unknown>")
         def _fset(self, value):
             old_value = fget(self)
+            fset(self, value)
+            change_detected = False
             if value != old_value:
+                change_detected = True
+            elif mutable == True:
+                if True: #hasattr(self, "_change_hook_property_mutable_cache_%s" % name):
+                    # compare cached string with new value
+                    #old_string = getattr(self, "_change_hook_property_mutable_cache_%s" % name)
+                    old_string = "dummy"
+                    #print "comparing", name, "mutable strings", old_string, repr(value)
+                    if repr(value) != old_string:
+                        change_detected = True
+            #print "testing", name, "change hook property", change_detected, value
+            if change_detected:
                 hook(self, old_value, value)
-            fset(self, value)
+            if mutable == True: # cache the new value for next time
+                setattr(self, "_change_hook_property_mutable_cache_%s" % name, repr(value))
         funcs["fset"] = _fset
         return funcs
     return decorator
diff --git a/libbe/settings_object.py b/libbe/settings_object.py
index 1df3e6b..3cbfdda 100644
--- a/libbe/settings_object.py
+++ b/libbe/settings_object.py
@@ -87,6 +87,7 @@ def attr_name_to_setting_name(self, name):
 def versioned_property(name, doc,
                        default=None, generator=None,
                        change_hook=prop_save_settings,
+                       mutable=False,
                        primer=prop_load_settings,
                        allowed=None, check_fn=None,
                        settings_properties=[],
@@ -137,7 +138,7 @@ def versioned_property(name, doc,
             checked = checked_property(allowed=allowed)
             fulldoc += "\n\nThe allowed values for this property are: %s." \
                        % (', '.join(allowed))
-        hooked      = change_hook_property(hook=change_hook)
+        hooked      = change_hook_property(hook=change_hook, mutable=mutable)
         primed      = primed_property(primer=primer, initVal=UNPRIMED)
         settings    = settings_property(name=name, null=UNPRIMED)
         docp        = doc_property(doc=fulldoc)
-- 
2.26.2