[brisbane-core/MERGE] minor CHKMap clean-ups

[John Arbash Meinel]

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

Ian Clatworthy wrote:
> I did a partial review of CHKMap.py today trying to
> track down a bug (bzr ls -r-1 aren't working yet).
> Attached are some tweaks, mostly in comments, but some
> code fixes as well.
> 
> I hope the code fixes are obvious. The only one that isn't
> is the removal of a redundant __repr__ routine - the
> superclass has identical code so it's not needed in the
> subclass.
> 
> Ian C.
> 
> PS: If we think this code is now stable enough to come into
> bzr.dev, I'll do a more formal review.
> 


@@ -148,12 +146,13 @@
             # Demand-load the root
             self._root_node = self._get_node(self._root_node)
             # XXX: Shouldn't this be put into _deserialize?
+            # IGC: I'm pretty sure this next line is redundant and can go
             self._root_node._search_key_func = self._search_key_func

^- This is redundant with _get_node(), we can remove it, and the XXX:

...

@@ -200,11 +199,13 @@

include_keys=include_keys))
         else:
             for key, value in sorted(node._items.iteritems()):
+                # IGC: Shouldn't prefix and indent be used below?
                 result.append('      %r %r' % (key, value))
         return result

^- (This is in dump_tree, I believe)

I intentionally didn't very the indenting for keys. The reason is so
that we would have something to 'line up' on when doing
'assertEqualDiff'. So that if the overall structure changed, it would be
easier to see which bits changed, and which bits did not. For example,
consider:

LeafNode
  a
  b
  c

becoming

InternalNode
  LeafNode
    a
  LeafNode
    b
  LeafNode
    c

If you change indenting, the whole block shows up as different, if you
leave keys at a fixed indent, then only the addition of internal
structure is changed.

I don't have an enormous preference, but that is the reason. Oh, and I
certainly don't think we want 'prefix' here, because I think it just
clutters the output.

...

         if len(bytes) != result._current_size():
- -            import pdb; pdb.set_trace()
+            #import pdb; pdb.set_trace()
             raise AssertionError('_current_size computed incorrectly')

^- The pdb was intentional, but if we want to remove them, we should
strip them completely, and not just comment them out.


Otherwise it seems fine, and certainly is reasonable to land.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmn5noACgkQJdeBCYSNAAMFfgCguno8G9m5/8TaUFsMPfMdZiVL
Ba4AnipKBRhceoYVEf+fqrnltxpnz2Ef
=e6cn
-----END PGP SIGNATURE-----