Merge lp:~gz/bzrtools/dot_encoding_211104 into lp:bzrtools

[Martin Packman]

Get the dot encoding correct at the graph definition level rather than leaving it till writing to the subprocess pipe. This is needed because invoke_dot_html feeds an output file of dot with layout annotations back in as input to save calculating the layout twice, and would otherwise have to decode it in order to encode it again.

While there, made the writer function accept a filename string as input as an alternative to an iterable of strings, which saves some file lifetime problems in invoke_dot_html as well.

The problem is nice and easy to see using just the bzrtools branch thanks to Benoît Pierre appearing.

[Richard Wilbur (richard-wilbur)]

Looks like this is very likely just as relevant as when Martin first proposed merging this branch according to a recent comment on the associated bug.[0]

Good work, Martin!
+1

[0] https://bugs.launchpad.net/bzrtools/+bug/211104/comments/3

review: Approve

Unmerged revisions

783. By Martin Packman on 2011-12-04

Allow passing a filename to invoke_dot which helps get file lifetimes correct

782. By Martin Packman on 2011-12-04

Ensure dot is encoded correctly at definition time rather than just before writing to subprocess

=== modified file 'dotgraph.py'
--- dotgraph.py	2008-04-11 00:03:51 +0000
+++ dotgraph.py	2011-12-04 18:30:39 +0000
@@ -69,11 +69,11 @@
             attributes.append('style="%s"' % ",".join(style))
         label = self.label
         if label is not None:
-            attributes.append('label="%s"' % label)
+            attributes.append('label="%s"' % label.encode('utf-8'))
         attributes.append('shape="box"')
         tooltip = None
         if self.message is not None:
-            tooltip = self.message
+            tooltip = self.message.encode('utf-8')
         attributes.append(self.get_attribute('tooltip', tooltip))
         if self.href is not None:
             attributes.append('href="%s"' % self.href)
@@ -210,16 +210,22 @@
                '-Efontsize=%d' % fontsize]
     if out_file is not None:
         cmdline.extend(('-o', out_file))
+    if isinstance(input, basestring):
+        cmdline.append(input)
+        stdin = None
+    else:
+        stdin = PIPE
     try:
-        dot_proc = Popen(cmdline, stdin=PIPE)
+        dot_proc = Popen(cmdline, stdin=stdin)
     except OSError, e:
         if e.errno == errno.ENOENT:
             raise NoDot()
         else:
             raise
-    for line in input:
-        dot_proc.stdin.write(line.encode('utf-8'))
-    dot_proc.stdin.close()
+    if stdin == PIPE:
+        for line in input:
+            dot_proc.stdin.write(line)
+        dot_proc.stdin.close()
     return dot_proc.wait()
 
 def invoke_dot_html(input, out_file):
@@ -232,13 +238,11 @@
         temp_dot = os.path.join(tempdir, 'temp.dot')
         status = invoke_dot(input, temp_dot, file_type='dot')
 
-        dot = open(temp_dot)
         temp_file = os.path.join(tempdir, 'temp.cmapx')
-        status = invoke_dot(dot, temp_file, 'cmapx')
+        status = invoke_dot(temp_dot, temp_file, 'cmapx')
 
         png_file = '.'.join(out_file.split('.')[:-1] + ['png'])
-        dot.seek(0)
-        status = invoke_dot(dot, png_file, 'png')
+        status = invoke_dot(temp_dot, png_file, 'png')
 
         png_relative = png_file.split('/')[-1]
         html = open(out_file, 'wb')