[MERGE] bzrlib API to add revision properties to a working tree

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

James Henstridge wrote:
> Attached is a bundle of my working tree revision properties branch.
> 
> I've fixed the revision property name issues to ensure that they are
> ASCII compatible in addition to the current restriction on them not
> containing whitespace.  The sanity check on property names is also
> performed in WorkingTree.set_revision_properties(), to catch errors
> like this early.
> 
> I've added tests for non-ASCII revision property names and values
> (that the first raise exceptions and the second are preserved).
> 
> I also added a TODO item for registering callbacks to be called during
> commit(), as requested by John.
> 
> James.
> 


> +class TestRevisionProperties(TestCaseWithWorkingTree):
> +
> +    def test_get_revision_properties(self):
> +        t = self.make_branch_and_tree('.')
> +        self.assertEqual({}, t.get_revision_properties())
> +        t.set_revision_properties({'key1': u'value1'})
> +        self.assertEqual({'key1': u'value1'}, t.get_revision_properties())
> +        t.add_revision_property('key2', 'value2')
> +        self.assertEqual({'key1': u'value1',
> +                          'key2': u'value2'}, t.get_revision_properties())
> +        t.add_revision_property('name', u'Erik B\xe5gfors')
> +        self.assertEqual(u'Erik B\xe5gfors',
> +                         t.get_revision_properties()['name'])
> +        self.assertRaises(ValueError,
> +                          t.add_revision_property,
> +                          'n\xC3\xA2me', 'value')

^- Why did you use a utf-8 string, rather than a Unicode string?


...

> +def check_properties(properties):
> +    """Verify that a dictionary of revision properties are OK."""
> +    for name, value in properties.iteritems():
> +        if not isinstance(name, basestring) or contains_whitespace(name):
> +            raise ValueError("invalid property name %r" % name)
> +        try:
> +            name.encode('ASCII')
> +        except UnicodeError:
> +            raise ValueError("property name %r is not representable "
> +                             "in ASCII" % name)
> +        if not isinstance(value, basestring):
> +            raise ValueError("invalid property value %r for %r" % 
> +                             (name, value))
> +
>  def is_ancestor(revision_id, candidate_id, branch):
>      """Return true if candidate_id is an ancestor of revision_id.

...

>  MERGE_MODIFIED_HEADER_1 = "BZR merge-modified list format 1"
>  CONFLICT_HEADER_1 = "BZR conflict list format 1"
> +REVISION_PROPERTIES_HEADER_1 = "BZR revision properties list format 1"

^- I realize you are following the crowd here, but I thought we
preferred using the name Bazaar over BZR. (At least you aren't using
Bazaar-NG).


So a couple small comments, but you still have my +1. (I would like BZR
=> Bazaar to be resolved, since this is a format on disk, so it lasts a
while).

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFAETqJdeBCYSNAAMRAu/BAKDKKggdV1+XpAq5q+EZtIPDA8wWqgCfSj6j
U2FA5QBWGoRtFrZCW7Om4LU=
=GdhY
-----END PGP SIGNATURE-----