Merge lp:~spud/spud/copy-paste-fix into lp:spud
- copy-paste-fix
- Merge into trunk
Status: | Superseded |
---|---|
Proposed branch: | lp:~spud/spud/copy-paste-fix |
Merge into: | lp:spud |
Diff against target: |
412 lines (+83/-91) 4 files modified
diamond/diamond/choice.py (+18/-32) diamond/diamond/interface.py (+53/-33) diamond/diamond/schema.py (+10/-16) diamond/diamond/tree.py (+2/-10) |
To merge this branch: | bzr merge lp:~spud/spud/copy-paste-fix |
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Cian Wilson | Needs Information | ||
Patrick Farrell | Pending | ||
Review via email:
|
This proposal has been superseded by a proposal from 2011-07-27.
Commit message
Description of the change
Fix for bug #811200. on_paste was not handling MixedTree's in the same way as on_copy.

Patrick Farrell (pefarrell) wrote : | # |
I attempted to replicate #811200 with this branch.
I don't get Cian's terminal error; I think that's been fixed, and I suspect it's something to do with running the wrong version of diamond. Cian: when you check out a branch, just do
cd branch
./configure
python diamond/bin/diamond [args]
and it will automatically use the right code.
But I get a different bug. It doesn't give any exception, but when I save the file I get
<?xml version='1.0' encoding='utf-8'?>
<dummy_options>
<system name="Dummy">
<nonlinear_
<type name="SNES">
<python name="Jacobian" rank="1">
</python>
</type>
</nonlinear
<nonlinear_
<type name="SNES">
<python name="Jacobian" rank="1">
</python>
</type>
</nonlinear
</system>
</dummy_options>
Note that the second nonlinear solver has a) lost its name, and b) lost its python code. It *looks* fine in the live diamond window, but if you save it it gets lost. (If you open it up again, you'll get all sorts of lost element errors.)

Cian Wilson (cwilson) wrote : | # |
> cd branch
> ./configure
> python diamond/bin/diamond [args]
I get the same error (terminal output and all) running the command this way:
cwilson@
Just to confirm it's the branch I'm running:
cwilson@
Repository tree (format: 2a)
Location:
shared repository: /home/cwilson/
repository branch: .
Related branches:
parent branch: bzr+ssh:
> <?xml version='1.0' encoding='utf-8'?>
> <dummy_options>
> <system name="Dummy">
> <nonlinear_solver name="Simple">
> <type name="SNES">
> <python name="Jacobian" rank="1">
> <string_value lines="20" type="python">some python code
> here</string_value>
> </python>
> </type>
> </nonlinear_solver>
> <nonlinear_solver>
> <type name="SNES">
> <python name="Jacobian" rank="1">
> <string_value type="python" lines="20"/>
> </python>
> </type>
> </nonlinear_solver>
> </system>
> </dummy_options>
>
> Note that the second nonlinear solver has a) lost its name, and b) lost its
> python code. It *looks* fine in the live diamond window, but if you save it it
> gets lost. (If you open it up again, you'll get all sorts of lost element
> errors.)
With the exception of looking fine in the diamond window before I close it (i.e. it doesn't look like it's copied to me and diamond shows elements as blue still) that sounds like the same behaviour I see on reopening the file.

Patrick Farrell (pefarrell) wrote : | # |
Also: I seem to get different behaviour depending on whether I CTRL+C/CTRL+V, or right-click copy, right-click paste. When I right-click copy, I get the following error in the terminal:
Traceback (most recent call last):
File "/data/
if widget is not self.treeview and gobject.
TypeError: type must be instantiable or an interface
The XML appears correct regardless of which approach is taken.
When I paste onto the blank nonlinear_solver, it doesn't have the right cardinality: it doesn't have the '-' on the right-hand side of the treeview to deactivate it.

Cian Wilson (cwilson) wrote : | # |
The above was done by first adding a new nonlinear_solver element then running CTRL+C on the old copy nonlinear_
If instead I use the right-click feature I get:
Traceback (most recent call last):
File "/home/
if widget is not self.treeview and gobject.
TypeError: type must be instantiable or an interface
on copy and:
Traceback (most recent call last):
File "/home/
if widget is not self.treeview and gobject.
TypeError: type must be instantiable or an interface
on paste.

Fraser Waters (fraser-waters08) wrote : | # |
"TypeError: type must be instantiable or an interface"
I found the cause of that, it's fixed in my local version.
The rest I'm not sure, I'll look into it more it's probably some confusion between Choices, Trees and MixedTrees. Really need to standardize them at some point.
- 419. By Fraser Waters
-
Small change to tree/choice write_core.
- 420. By Fraser Waters
-
Fix ups
- 421. By Fraser Waters
-
Merge
- 422. By Fraser Waters
-
Can reopen current file.
- 423. By Fraser Waters
-
Merge from trunk
- 424. By Fraser Waters
-
Paste needs active node
- 425. By Fraser Waters
-
Factor out handling clipboard for other widgets.
- 426. By Fraser Waters
-
Missed a :
- 427. By Fraser Waters
-
Renamed Choice.l to Choice.choices
- 428. By Fraser Waters
-
Fix for paste handling choices.
- 429. By Fraser Waters
-
non xml paste will not ask to delete trees.
- 430. By Fraser Waters
-
Start on changing schema.read
- 431. By Fraser Waters
-
Merge from trunk.
- 432. By Fraser Waters
-
Schema.read handles starting on a Choice
Unmerged revisions
Preview Diff
1 | === modified file 'diamond/diamond/choice.py' |
2 | --- diamond/diamond/choice.py 2011-07-26 14:04:07 +0000 |
3 | +++ diamond/diamond/choice.py 2011-07-27 13:28:56 +0000 |
4 | @@ -31,21 +31,21 @@ |
5 | __gsignals__ = { "on-set-data" : (gobject.SIGNAL_RUN_LAST, gobject.TYPE_NONE, (str,)), |
6 | "on-set-attr" : (gobject.SIGNAL_RUN_LAST, gobject.TYPE_NONE, (str, str))} |
7 | |
8 | - def __init__(self, l, cardinality=''): |
9 | + def __init__(self, choices, cardinality=''): |
10 | gobject.GObject.__init__(self) |
11 | |
12 | - self.l = l |
13 | - if l == []: |
14 | + self.choices = choices |
15 | + if choices == []: |
16 | raise Exception |
17 | self.index = 0 |
18 | name = "" |
19 | - for choice in l: |
20 | + for choice in choices: |
21 | assert choice.__class__ is tree.Tree |
22 | - name = name + choice.name + ":" |
23 | choice.connect("on-set-data", self._on_set_data) |
24 | choice.connect("on-set-attr", self._on_set_attr) |
25 | |
26 | - name = name[:-1] |
27 | + name = ":".join(choice.name for choice in choices) |
28 | + |
29 | self.name = name |
30 | self.schemaname = name |
31 | self.cardinality = cardinality |
32 | @@ -67,21 +67,21 @@ |
33 | self.index = i |
34 | |
35 | def find_tree(self, name): |
36 | - for t in self.l: |
37 | + for t in self.choices: |
38 | if t.name == name: |
39 | return t |
40 | |
41 | debug.deprint("self.name == %s" % self.name, 0) |
42 | - for choice in self.l: |
43 | + for choice in self.choices: |
44 | debug.deprint("choice.name == %s" % choice.name, 0) |
45 | raise Exception, "No such choice name: %s" % name |
46 | |
47 | def set_active_choice_by_name(self, name): |
48 | matched = False |
49 | - for t in self.l: |
50 | + for t in self.choices: |
51 | if t.name == name.strip(): |
52 | matched = True |
53 | - self.index = self.l.index(t) |
54 | + self.index = self.choices.index(t) |
55 | |
56 | if not matched: |
57 | raise Exception, "no such name %s found" % name |
58 | @@ -89,11 +89,11 @@ |
59 | self.recompute_validity() |
60 | |
61 | def set_active_choice_by_ref(self, ref): |
62 | - self.index = self.l.index(ref) |
63 | + self.index = self.choices.index(ref) |
64 | self.recompute_validity() |
65 | |
66 | def get_current_tree(self): |
67 | - return self.l[self.index] |
68 | + return self.choices[self.index] |
69 | |
70 | def add_children(self, schema): |
71 | return self.get_current_tree().add_children(schema) |
72 | @@ -106,7 +106,7 @@ |
73 | |
74 | def copy(self): |
75 | new_choices = [] |
76 | - for choice in self.l: |
77 | + for choice in self.choices: |
78 | new_choices.append(choice.copy()) |
79 | |
80 | new_choice = Choice(new_choices) |
81 | @@ -114,37 +114,23 @@ |
82 | setattr(new_choice, attr, copy.copy(getattr(self, attr))) |
83 | |
84 | new_choice.set_parent(self.parent) |
85 | - for choice in new_choice.l: |
86 | + for choice in new_choice.choices: |
87 | choice.children = copy.copy([]) |
88 | |
89 | return new_choice |
90 | |
91 | def get_possible_names(self): |
92 | - return [x.name for x in self.l] |
93 | + return [x.name for x in self.choices] |
94 | |
95 | def set_parent(self, parent): |
96 | self.parent = parent |
97 | - for choice in self.l: |
98 | + for choice in self.choices: |
99 | choice.parent = parent |
100 | |
101 | def write_core(self, parent): |
102 | - l = self.l |
103 | - for i in range(len(l)): |
104 | - if self.index == i: |
105 | - l[i].write_core(parent) |
106 | -# else: |
107 | -# root=etree.Element(parent.tag) |
108 | -# l[i].write_core(root) |
109 | -# comment_buffer = StringIO.StringIO(etree.tostring(root)) |
110 | -# comment_text = ("DIAMOND MAGIC COMMENT (neglected choice subtree %s):\n" % l[i].schemaname) |
111 | -# comment_text = comment_text + base64.b64encode(bz2.compress(comment_buffer.getvalue())) |
112 | -# parent.append(etree.Comment(unicode(comment_text))) |
113 | - |
114 | + self.choices[self.index].write_core(parent) |
115 | return parent |
116 | |
117 | - def choices(self): |
118 | - return self.l |
119 | - |
120 | def is_comment(self): |
121 | return False |
122 | |
123 | @@ -167,7 +153,7 @@ |
124 | return [self.get_current_tree()] |
125 | |
126 | def get_choices(self): |
127 | - return self.l |
128 | + return self.choices |
129 | |
130 | def is_hidden(self): |
131 | """ |
132 | |
133 | === modified file 'diamond/diamond/interface.py' |
134 | --- diamond/diamond/interface.py 2011-07-26 16:12:33 +0000 |
135 | +++ diamond/diamond/interface.py 2011-07-27 13:28:56 +0000 |
136 | @@ -283,9 +283,6 @@ |
137 | # if we have a relative path, make it absolute |
138 | filename = os.path.abspath(filename) |
139 | |
140 | - if filename == self.filename: |
141 | - return |
142 | - |
143 | try: |
144 | os.stat(filename) |
145 | except OSError: |
146 | @@ -638,12 +635,19 @@ |
147 | else: |
148 | return self._get_focus_widget(focus) |
149 | |
150 | + def _handle_clipboard(self, widget, signal): |
151 | + if isinstance(widget, gtk.MenuItem): |
152 | + return False |
153 | + else: |
154 | + widget = self._get_focus_widget(self.main_window) |
155 | + if widget is not self.treeview and gobject.signal_lookup(signal + "-clipboard", widget): |
156 | + widget.emit(signal + "-clipboard") |
157 | + return True |
158 | + return False |
159 | + |
160 | def on_copy(self, widget=None): |
161 | - if not isinstance(widget, gtk.MenuItem): |
162 | - widget = self._get_focus_widget(self.main_window) |
163 | - if widget is not self.treeview and gobject.signal_lookup("copy-clipboard", widget): |
164 | - widget.emit("copy-clipboard") |
165 | - return |
166 | + if self._handle_clipboard(widget, "copy"): |
167 | + return |
168 | |
169 | if isinstance(self.selected_node, mixedtree.MixedTree): |
170 | node = self.selected_node.parent |
171 | @@ -662,28 +666,47 @@ |
172 | return |
173 | |
174 | def on_paste(self, widget=None): |
175 | - if not isinstance(widget, gtk.MenuItem): |
176 | - widget = self._get_focus_widget(self.main_window) |
177 | - if widget is not self.treeview and gobject.signal_lookup("paste-clipboard", widget): |
178 | - widget.emit("paste-clipboard") |
179 | - return |
180 | + if self._handle_clipboard(widget, "paste"): |
181 | + return |
182 | |
183 | clipboard = gtk.clipboard_get() |
184 | ios = StringIO.StringIO(clipboard.wait_for_text()) |
185 | |
186 | if self.selected_iter is not None: |
187 | - node = self.treestore.get_value(self.selected_iter, 1) |
188 | + node = self.treestore.get_value(self.selected_iter, 0) |
189 | + oldtree = self.treestore.get_value(self.selected_iter, 1) |
190 | |
191 | if node != None: |
192 | - |
193 | - newnode = self.s.read(ios, node) |
194 | - |
195 | - if newnode is None: |
196 | + |
197 | + expand = not node.active |
198 | + if expand: |
199 | + self.expand_tree(self.selected_iter) |
200 | + |
201 | + newtree = self.s.read(ios, oldtree) |
202 | + |
203 | + if newtree is None: |
204 | + if expand: |
205 | + self.collapse_tree(self.selected_iter, False) |
206 | self.statusbar.set_statusbar("Trying to paste invalid XML.") |
207 | return |
208 | |
209 | - if not node.active: |
210 | - self.expand_tree(self.selected_iter) |
211 | + if oldtree.parent is not None: |
212 | + newtree.set_parent(oldtree.parent) |
213 | + if isinstance(node, tree.Tree): |
214 | + children = node.parent.get_children() |
215 | + children.insert(children.index(oldtree), newtree) |
216 | + children.remove(oldtree) |
217 | + |
218 | + if isinstance(node, choice.Choice): |
219 | + choices = node.get_choices() |
220 | + choices.insert(choices.index(oldtree), newtree) |
221 | + choices.remove(oldtree) |
222 | + self.set_treestore(self.selected_iter, [node], True, True) |
223 | + else: |
224 | + self.set_treestore(self.selected_iter, [newtree], True, True) |
225 | + |
226 | + newtree.recompute_validity() |
227 | + self.treeview.queue_draw() |
228 | |
229 | # Extract and display validation errors |
230 | lost_eles, added_eles, lost_attrs, added_attrs = self.s.read_errors() |
231 | @@ -710,12 +733,6 @@ |
232 | dialogs.long_message(self.main_window, msg) |
233 | |
234 | self.set_saved(False) |
235 | - |
236 | - self.treeview.freeze_child_notify() |
237 | - iter = self.set_treestore(self.selected_iter, [newnode], True, True) |
238 | - self.treeview.thaw_child_notify() |
239 | - |
240 | - self.treeview.get_selection().select_iter(iter) |
241 | |
242 | return |
243 | |
244 | @@ -907,7 +924,7 @@ |
245 | if replace: |
246 | replacediter = iter |
247 | iter = self.treestore.iter_parent(replacediter) |
248 | - else: |
249 | + else: |
250 | self.remove_children(iter) |
251 | |
252 | for t in new_tree: |
253 | @@ -1119,7 +1136,7 @@ |
254 | |
255 | return |
256 | |
257 | - def collapse_tree(self, iter): |
258 | + def collapse_tree(self, iter, confirm = True): |
259 | """ |
260 | Collapses part of the tree. |
261 | """ |
262 | @@ -1147,7 +1164,7 @@ |
263 | self.set_saved(False) |
264 | self.remove_children(iter) |
265 | else: |
266 | - self.delete_tree(iter) |
267 | + self.delete_tree(iter, confirm) |
268 | |
269 | elif choice_or_tree.cardinality == "+": |
270 | count = parent_tree.count_children_by_schemaname(choice_or_tree.schemaname) |
271 | @@ -1155,20 +1172,23 @@ |
272 | # do nothing |
273 | return |
274 | else: # count > 2 |
275 | - self.delete_tree(iter) |
276 | + self.delete_tree(iter, confirm) |
277 | |
278 | parent_tree.recompute_validity() |
279 | self.treeview.queue_draw() |
280 | return |
281 | |
282 | - def delete_tree(self, iter): |
283 | + def delete_tree(self, iter, confirm): |
284 | choice_or_tree, = self.treestore.get(iter, 0) |
285 | parent_tree = choice_or_tree.parent |
286 | isSelected = self.treeview.get_selection().iter_is_selected(iter) |
287 | sibling = self.treestore.iter_next(iter) |
288 | |
289 | - confirm = dialogs.prompt(self.main_window, "Are you sure you want to delete this node?") |
290 | - if confirm == gtk.RESPONSE_YES: |
291 | + if confirm: |
292 | + response = dialogs.prompt(self.main_window, "Are you sure you want to delete this node?") |
293 | + |
294 | + # not A or B == A implies B |
295 | + if not confirm or response == gtk.RESPONSE_YES: |
296 | parent_tree.delete_child_by_ref(choice_or_tree) |
297 | self.remove_children(iter) |
298 | self.treestore.remove(iter) |
299 | |
300 | === modified file 'diamond/diamond/schema.py' |
301 | --- diamond/diamond/schema.py 2011-07-18 11:49:27 +0000 |
302 | +++ diamond/diamond/schema.py 2011-07-27 13:28:56 +0000 |
303 | @@ -162,12 +162,9 @@ |
304 | node = self.to_tree(node) |
305 | |
306 | if eidtree is not None: |
307 | - if eidtree.parent is not None: |
308 | - eidtree.parent.children.append(node) |
309 | - eidtree.parent.children.remove(eidtree) |
310 | - node.set_parent(eidtree.parent) |
311 | node.attrs = eidtree.attrs |
312 | node.cardinality = eidtree.cardinality |
313 | + node.parent = eidtree.parent |
314 | |
315 | return node |
316 | |
317 | @@ -395,14 +392,11 @@ |
318 | # bloody simplified RNG |
319 | if len(children) == 2: |
320 | empty = [x for x in children if self.tag(x) == "empty"] |
321 | - nonempty = [x for x in children if self.tag(x) != "empty"] |
322 | - if len(empty) > 0: |
323 | + if empty: |
324 | + nonempty = [x for x in children if self.tag(x) != "empty"] |
325 | tag = self.tag(nonempty[0]) |
326 | - if tag == "oneOrMore": |
327 | - return self.cb_oneormore(element, facts) |
328 | - else: |
329 | - f = self.callbacks[tag] |
330 | - return f(element, facts) |
331 | + f = self.callbacks[tag] |
332 | + return f(element, facts) |
333 | |
334 | for child in children: |
335 | newfacts = {} |
336 | @@ -564,7 +558,7 @@ |
337 | xmlname = xmlnode.get("name") |
338 | have_found = False |
339 | |
340 | - possibles = [tree_choice for tree_choice in datatree.choices() if tree_choice.name == xmlnode.tag] |
341 | + possibles = [tree_choice for tree_choice in datatree.get_choices() if tree_choice.name == xmlnode.tag] |
342 | # first loop over the fixed-value names |
343 | for tree_choice in possibles: |
344 | if "name" not in tree_choice.attrs: |
345 | @@ -698,7 +692,7 @@ |
346 | |
347 | for schemachild in priority_queue: |
348 | if schemachild.cardinality in ['', '?']: |
349 | - for curtree in schemachild.choices(): |
350 | + for curtree in schemachild.get_choices(): |
351 | name = curtree.name |
352 | |
353 | have_fixed_name = False |
354 | @@ -725,7 +719,7 @@ |
355 | xmls[schemachild.schemaname] = copy.deepcopy([]) |
356 | elif schemachild.cardinality in ['*', '+']: |
357 | xmls[schemachild.schemaname] = copy.deepcopy([]) |
358 | - for curtree in schemachild.choices(): |
359 | + for curtree in schemachild.get_choices(): |
360 | name = curtree.name |
361 | |
362 | have_fixed_name = False |
363 | @@ -803,7 +797,7 @@ |
364 | bins[schemachild.schemaname].append(child) |
365 | |
366 | # search for neglected choices |
367 | - if schemachild.__class__ is choice.Choice and schemachild.cardinality in ['', '?']: |
368 | + if isinstance(schemachild, choice.Choice) and schemachild.cardinality in ['', '?']: |
369 | for child in bins[schemachild.schemaname]: |
370 | |
371 | # Does the child have a valid XML node attached? |
372 | @@ -811,7 +805,7 @@ |
373 | if child.xmlnode is None: continue |
374 | |
375 | current_choice = child.get_current_tree() |
376 | - for tree_choice in child.l: |
377 | + for tree_choice in child.get_choices(): |
378 | if tree_choice is current_choice: continue |
379 | |
380 | return bins |
381 | |
382 | === modified file 'diamond/diamond/tree.py' |
383 | --- diamond/diamond/tree.py 2011-07-26 14:04:07 +0000 |
384 | +++ diamond/diamond/tree.py 2011-07-27 13:28:56 +0000 |
385 | @@ -266,7 +266,7 @@ |
386 | |
387 | sub_tree=etree.Element(self.name) |
388 | |
389 | - for key in self.attrs.keys(): |
390 | + for key in self.attrs: |
391 | val = self.attrs[key] |
392 | output_val = val[1] |
393 | if output_val is not None: |
394 | @@ -275,17 +275,9 @@ |
395 | for child in self.children: |
396 | if child.active is True: |
397 | child.write_core(sub_tree) |
398 | -# else: |
399 | -# if child.cardinality == '?': |
400 | -# root=etree.Element(self.name) |
401 | -# child.write_core(root) |
402 | -# comment_buffer = StringIO.StringIO(etree.tostring(root)) |
403 | -# comment_text = ("DIAMOND MAGIC COMMENT (inactive optional subtree %s):\n" % child.schemaname) |
404 | -# comment_text = comment_text + base64.b64encode(bz2.compress(comment_buffer.getvalue())) |
405 | -# sub_tree.append(etree.Comment(unicode(comment_text))) |
406 | |
407 | if self.data is not None: |
408 | - sub_tree.text=(unicode(self.data)) |
409 | + sub_tree.text = unicode(self.data) |
410 | |
411 | if parent is not None: |
412 | parent.append(sub_tree) |
With that branch I'm still getting the same error:
File "/usr/lib/ python2. 7/dist- packages/ diamond/ interface. py", line 695, in on_paste python2. 7/dist- packages/ diamond/ schema. py", line 532, in read node(root) python2. 7/dist- packages/ diamond/ schema. py", line 167, in valid_node parent. children. remove( eidtree)
newnode = self.s.read(ios, node)
File "/usr/lib/
datatree = self.valid_
File "/usr/lib/
eidtree.
ValueError: list.remove(x): x not in list
and behaviour as reported in bug #811200 (i.e. saving the file immediately afterwards saves a file that's different from the one you're seeing and you also don't get a copied branch of the tree).
Can anyone else reproduce this or have I mangled my paths somewhere? (I packaged the branch and installed it so shouldn't be mixing versions of diamond.)