Rev 5907: (gz) Fix unicode handling in bzrlib.conflicts ui code (Martin [gz]) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Sat May 21 17:50:54 UTC 2011


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5907 [merge]
revision-id: pqm at pqm.ubuntu.com-20110521175049-yz1fb5qcgrkhz2d0
parent: pqm at pqm.ubuntu.com-20110520212809-ugt47o73t2ntwbmw
parent: gzlist at googlemail.com-20110521164509-iez0bl2i0400xzqq
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Sat 2011-05-21 17:50:49 +0000
message:
  (gz) Fix unicode handling in bzrlib.conflicts ui code (Martin [gz])
modified:
  bzrlib/conflicts.py            conflicts.py-20051001061850-78ef952ba63d2b42
  bzrlib/merge.py                merge.py-20050513021216-953b65a438527106
  bzrlib/status.py               status.py-20050505062338-431bfa63ec9b19e6
  bzrlib/tests/blackbox/test_conflicts.py test_conflicts.py-20060228151432-9723ebb925b999cf
  bzrlib/tests/test_conflicts.py test_conflicts.py-20051006031059-e2dad9bbeaa5891f
  bzrlib/tests/test_transform.py test_transaction.py-20060105172520-b3ffb3946550e6c4
  bzrlib/transform.py            transform.py-20060105172343-dd99e54394d91687
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/conflicts.py'
--- a/bzrlib/conflicts.py	2011-02-08 13:56:49 +0000
+++ b/bzrlib/conflicts.py	2011-05-21 16:43:19 +0000
@@ -72,7 +72,7 @@
                     continue
                 self.outf.write(conflict.path + '\n')
             else:
-                self.outf.write(str(conflict) + '\n')
+                self.outf.write(unicode(conflict) + '\n')
 
 
 resolve_action_registry = registry.Registry()
@@ -148,7 +148,7 @@
                     trace.note('%d conflict(s) auto-resolved.', len(resolved))
                     trace.note('Remaining conflicts:')
                     for conflict in un_resolved:
-                        trace.note(conflict)
+                        trace.note(unicode(conflict))
                     return 1
                 else:
                     trace.note('All conflicts resolved.')
@@ -291,7 +291,7 @@
     def to_strings(self):
         """Generate strings for the provided conflicts"""
         for conflict in self:
-            yield str(conflict)
+            yield unicode(conflict)
 
     def remove_files(self, tree):
         """Remove the THIS, BASE and OTHER files for listed conflicts"""
@@ -390,7 +390,7 @@
     def __ne__(self, other):
         return not self.__eq__(other)
 
-    def __str__(self):
+    def __unicode__(self):
         return self.format % self.__dict__
 
     def __repr__(self):

=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py	2011-05-18 16:42:48 +0000
+++ b/bzrlib/merge.py	2011-05-21 16:43:19 +0000
@@ -888,7 +888,7 @@
                 self.tt.iter_changes(), self.change_reporter)
         self.cook_conflicts(fs_conflicts)
         for conflict in self.cooked_conflicts:
-            trace.warning(conflict)
+            trace.warning(unicode(conflict))
 
     def _entries3(self):
         """Gather data about files modified between three trees.

=== modified file 'bzrlib/status.py'
--- a/bzrlib/status.py	2011-03-30 11:50:40 +0000
+++ b/bzrlib/status.py	2011-05-21 16:43:19 +0000
@@ -194,7 +194,7 @@
                     prefix = 'C  '
                 else:
                     prefix = ' '
-                to_file.write("%s %s\n" % (prefix, conflict))
+                to_file.write("%s %s\n" % (prefix, unicode(conflict)))
             # Show files that were requested but don't exist (and are
             # not versioned).  We don't involve delta in this; these
             # paths are really the province of just the status

=== modified file 'bzrlib/tests/blackbox/test_conflicts.py'
--- a/bzrlib/tests/blackbox/test_conflicts.py	2010-11-07 16:32:51 +0000
+++ b/bzrlib/tests/blackbox/test_conflicts.py	2011-05-21 16:29:38 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006, 2007, 2009, 2010 Canonical Ltd
+# Copyright (C) 2006, 2007, 2009, 2010, 2011 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -21,29 +21,31 @@
     )
 from bzrlib.tests import script
 
-def make_tree_with_conflicts(test, this_path='this', other_path='other'):
+
+def make_tree_with_conflicts(test, this_path='this', other_path='other',
+        prefix='my'):
     this_tree = test.make_branch_and_tree(this_path)
     test.build_tree_contents([
-        ('%s/myfile' % (this_path,), 'this content\n'),
-        ('%s/my_other_file' % (this_path,), 'this content\n'),
-        ('%s/mydir/' % (this_path,),),
+        ('%s/%sfile' % (this_path, prefix), 'this content\n'),
+        ('%s/%s_other_file' % (this_path, prefix), 'this content\n'),
+        ('%s/%sdir/' % (this_path, prefix),),
         ])
-    this_tree.add('myfile')
-    this_tree.add('my_other_file')
-    this_tree.add('mydir')
+    this_tree.add(prefix+'file')
+    this_tree.add(prefix+'_other_file')
+    this_tree.add(prefix+'dir')
     this_tree.commit(message="new")
     other_tree = this_tree.bzrdir.sprout(other_path).open_workingtree()
     test.build_tree_contents([
-        ('%s/myfile' % (other_path,), 'contentsb\n'),
-        ('%s/my_other_file' % (other_path,), 'contentsb\n'),
+        ('%s/%sfile' % (other_path, prefix), 'contentsb\n'),
+        ('%s/%s_other_file' % (other_path, prefix), 'contentsb\n'),
         ])
-    other_tree.rename_one('mydir', 'mydir2')
+    other_tree.rename_one(prefix+'dir', prefix+'dir2')
     other_tree.commit(message="change")
     test.build_tree_contents([
-        ('%s/myfile' % (this_path,), 'contentsa2\n'),
-        ('%s/my_other_file' % (this_path,), 'contentsa2\n'),
+        ('%s/%sfile' % (this_path, prefix), 'contentsa2\n'),
+        ('%s/%s_other_file' % (this_path, prefix), 'contentsa2\n'),
         ])
-    this_tree.rename_one('mydir', 'mydir3')
+    this_tree.rename_one(prefix+'dir', prefix+'dir3')
     this_tree.commit(message='change')
     this_tree.merge_from_branch(other_tree.branch)
     return this_tree, other_tree
@@ -79,3 +81,45 @@
 Path conflict: mydir3 / mydir2
 Text conflict in myfile
 """)
+
+
+class TestUnicodePaths(tests.TestCaseWithTransport):
+    """Unicode characters in conflicts should be displayed properly"""
+
+    encoding = "UTF-8"
+
+    def _as_output(self, text):
+        return text
+
+    def test_messages(self):
+        """Conflict messages involving non-ascii paths are displayed okay"""
+        make_tree_with_conflicts(self, "branch", prefix=u"\xA7")
+        out, err = self.run_bzr(["conflicts", "-d", "branch"],
+            encoding=self.encoding)
+        self.assertEqual(out.decode(self.encoding),
+            u"Text conflict in \xA7_other_file\n"
+            u"Path conflict: \xA7dir3 / \xA7dir2\n"
+            u"Text conflict in \xA7file\n")
+        self.assertEqual(err, "")
+
+    def test_text_conflict_paths(self):
+        """Text conflicts on non-ascii paths are displayed okay"""
+        make_tree_with_conflicts(self, "branch", prefix=u"\xA7")
+        out, err = self.run_bzr(["conflicts", "-d", "branch", "--text"],
+            encoding=self.encoding)
+        self.assertEqual(out.decode(self.encoding),
+            u"\xA7_other_file\n"
+            u"\xA7file\n")
+        self.assertEqual(err, "")
+
+
+class TestUnicodePathsOnAsciiTerminal(TestUnicodePaths):
+    """Undisplayable unicode characters in conflicts should be escaped"""
+
+    encoding = "ascii"
+
+    def setUp(self):
+        self.skip("Need to decide if replacing is the desired behaviour")
+
+    def _as_output(self, text):
+        return text.encode(self.encoding, "replace")

=== modified file 'bzrlib/tests/test_conflicts.py'
--- a/bzrlib/tests/test_conflicts.py	2011-05-13 12:51:05 +0000
+++ b/bzrlib/tests/test_conflicts.py	2011-05-21 16:29:38 +0000
@@ -62,6 +62,11 @@
 ])
 
 
+def vary_by_conflicts():
+    for conflict in example_conflicts:
+        yield (conflict.__class__.__name__, {"conflict": conflict})
+
+
 class TestConflicts(tests.TestCaseWithTransport):
 
     def test_resolve_conflict_dir(self):
@@ -120,38 +125,57 @@
         self.assertEqual(conflicts.ConflictList([]), tree.conflicts())
 
 
-class TestConflictStanzas(tests.TestCase):
+class TestPerConflict(tests.TestCase):
+
+    scenarios = scenarios.multiply_scenarios(vary_by_conflicts())
+
+    def test_stringification(self):
+        text = unicode(self.conflict)
+        self.assertContainsString(text, self.conflict.path)
+        self.assertContainsString(text.lower(), "conflict")
+        self.assertContainsString(repr(self.conflict),
+            self.conflict.__class__.__name__)
 
     def test_stanza_roundtrip(self):
-        # write and read our example stanza.
-        stanza_iter = example_conflicts.to_stanzas()
-        processed = conflicts.ConflictList.from_stanzas(stanza_iter)
-        for o, p in zip(processed, example_conflicts):
-            self.assertEqual(o, p)
-
-            self.assertIsInstance(o.path, unicode)
-
-            if o.file_id is not None:
-                self.assertIsInstance(o.file_id, str)
-
-            conflict_path = getattr(o, 'conflict_path', None)
-            if conflict_path is not None:
-                self.assertIsInstance(conflict_path, unicode)
-
-            conflict_file_id = getattr(o, 'conflict_file_id', None)
-            if conflict_file_id is not None:
-                self.assertIsInstance(conflict_file_id, str)
+        p = self.conflict
+        o = conflicts.Conflict.factory(**p.as_stanza().as_dict())
+        self.assertEqual(o, p)
+
+        self.assertIsInstance(o.path, unicode)
+
+        if o.file_id is not None:
+            self.assertIsInstance(o.file_id, str)
+
+        conflict_path = getattr(o, 'conflict_path', None)
+        if conflict_path is not None:
+            self.assertIsInstance(conflict_path, unicode)
+
+        conflict_file_id = getattr(o, 'conflict_file_id', None)
+        if conflict_file_id is not None:
+            self.assertIsInstance(conflict_file_id, str)
 
     def test_stanzification(self):
-        for stanza in example_conflicts.to_stanzas():
-            if 'file_id' in stanza:
-                # In Stanza form, the file_id has to be unicode.
-                self.assertStartsWith(stanza['file_id'], u'\xeed')
-            self.assertStartsWith(stanza['path'], u'p\xe5th')
-            if 'conflict_path' in stanza:
-                self.assertStartsWith(stanza['conflict_path'], u'p\xe5th')
-            if 'conflict_file_id' in stanza:
-                self.assertStartsWith(stanza['conflict_file_id'], u'\xeed')
+        stanza = self.conflict.as_stanza()
+        if 'file_id' in stanza:
+            # In Stanza form, the file_id has to be unicode.
+            self.assertStartsWith(stanza['file_id'], u'\xeed')
+        self.assertStartsWith(stanza['path'], u'p\xe5th')
+        if 'conflict_path' in stanza:
+            self.assertStartsWith(stanza['conflict_path'], u'p\xe5th')
+        if 'conflict_file_id' in stanza:
+            self.assertStartsWith(stanza['conflict_file_id'], u'\xeed')
+
+
+class TestConflictList(tests.TestCase):
+
+    def test_stanzas_roundtrip(self):
+        stanzas_iter = example_conflicts.to_stanzas()
+        processed = conflicts.ConflictList.from_stanzas(stanzas_iter)
+        self.assertEqual(example_conflicts, processed)
+
+    def test_stringification(self):
+        for text, o in zip(example_conflicts.to_strings(), example_conflicts):
+            self.assertEqual(text, unicode(o))
 
 
 # FIXME: The shell-like tests should be converted to real whitebox tests... or

=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py	2011-05-13 12:51:05 +0000
+++ b/bzrlib/tests/test_transform.py	2011-05-21 16:44:52 +0000
@@ -811,7 +811,7 @@
         raw_conflicts = resolve_conflicts(tt)
         cooked_conflicts = cook_conflicts(raw_conflicts, tt)
         tt.finalize()
-        conflicts_s = [str(c) for c in cooked_conflicts]
+        conflicts_s = [unicode(c) for c in cooked_conflicts]
         self.assertEqual(len(cooked_conflicts), len(conflicts_s))
         self.assertEqual(conflicts_s[0], 'Conflict adding file dorothy.  '
                                          'Moved existing file to '

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2011-05-13 14:02:14 +0000
+++ b/bzrlib/transform.py	2011-05-21 16:43:19 +0000
@@ -2578,7 +2578,7 @@
             precomputed_delta = None
         conflicts = cook_conflicts(raw_conflicts, tt)
         for conflict in conflicts:
-            trace.warning(conflict)
+            trace.warning(unicode(conflict))
         try:
             wt.add_conflicts(conflicts)
         except errors.UnsupportedOperation:
@@ -2820,7 +2820,7 @@
                 unversioned_filter=working_tree.is_ignored)
             delta.report_changes(tt.iter_changes(), change_reporter)
         for conflict in conflicts:
-            trace.warning(conflict)
+            trace.warning(unicode(conflict))
         pp.next_phase()
         tt.apply()
         working_tree.set_merge_modified(merge_modified)

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-05-20 13:28:35 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-05-21 17:50:49 +0000
@@ -87,6 +87,9 @@
   submit_location public_location`` never sets ``submit_branch``
   nor ``public_branch``.  (Vincent Ladeuil)
 
+* Conflicts involving non-ascii filenames are now properly reported rather
+  than failing with a UnicodeEncodeError. (Martin [GZ], #686161)
+
 * Correct parent is now set when using 'switch -b' with bound branches.
   (A. S. Budden, #513709)
 




More information about the bazaar-commits mailing list