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

Proposed by Martin Packman on 2011-12-04
Status: Needs review
Proposed branch: lp:~gz/bzrtools/dot_encoding_211104
Merge into: lp:bzrtools
Diff against target: 60 lines (+14/-10)
1 file modified
dotgraph.py (+14/-10)
To merge this branch: bzr merge lp:~gz/bzrtools/dot_encoding_211104
Reviewer Review Type Date Requested Status
Richard Wilbur (community) Approve on 2014-01-17
Aaron Bentley 2011-12-04 Pending
Review via email: mp+84405@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Richard Wilbur (richard-wilbur) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'dotgraph.py'
2--- dotgraph.py 2008-04-11 00:03:51 +0000
3+++ dotgraph.py 2011-12-04 18:30:39 +0000
4@@ -69,11 +69,11 @@
5 attributes.append('style="%s"' % ",".join(style))
6 label = self.label
7 if label is not None:
8- attributes.append('label="%s"' % label)
9+ attributes.append('label="%s"' % label.encode('utf-8'))
10 attributes.append('shape="box"')
11 tooltip = None
12 if self.message is not None:
13- tooltip = self.message
14+ tooltip = self.message.encode('utf-8')
15 attributes.append(self.get_attribute('tooltip', tooltip))
16 if self.href is not None:
17 attributes.append('href="%s"' % self.href)
18@@ -210,16 +210,22 @@
19 '-Efontsize=%d' % fontsize]
20 if out_file is not None:
21 cmdline.extend(('-o', out_file))
22+ if isinstance(input, basestring):
23+ cmdline.append(input)
24+ stdin = None
25+ else:
26+ stdin = PIPE
27 try:
28- dot_proc = Popen(cmdline, stdin=PIPE)
29+ dot_proc = Popen(cmdline, stdin=stdin)
30 except OSError, e:
31 if e.errno == errno.ENOENT:
32 raise NoDot()
33 else:
34 raise
35- for line in input:
36- dot_proc.stdin.write(line.encode('utf-8'))
37- dot_proc.stdin.close()
38+ if stdin == PIPE:
39+ for line in input:
40+ dot_proc.stdin.write(line)
41+ dot_proc.stdin.close()
42 return dot_proc.wait()
43
44 def invoke_dot_html(input, out_file):
45@@ -232,13 +238,11 @@
46 temp_dot = os.path.join(tempdir, 'temp.dot')
47 status = invoke_dot(input, temp_dot, file_type='dot')
48
49- dot = open(temp_dot)
50 temp_file = os.path.join(tempdir, 'temp.cmapx')
51- status = invoke_dot(dot, temp_file, 'cmapx')
52+ status = invoke_dot(temp_dot, temp_file, 'cmapx')
53
54 png_file = '.'.join(out_file.split('.')[:-1] + ['png'])
55- dot.seek(0)
56- status = invoke_dot(dot, png_file, 'png')
57+ status = invoke_dot(temp_dot, png_file, 'png')
58
59 png_relative = png_file.split('/')[-1]
60 html = open(out_file, 'wb')

Subscribers

People subscribed via source and target branches