Merge lp:~cwilson/spud/lines_attribute_checkpoint_fix into lp:spud

Proposed by Cian Wilson
Status: Rejected
Rejected by: Cian Wilson
Proposed branch: lp:~cwilson/spud/lines_attribute_checkpoint_fix
Merge into: lp:spud
Diff against target: 19 lines (+2/-0)
1 file modified
src/spud.cpp (+2/-0)
To merge this branch: bzr merge lp:~cwilson/spud/lines_attribute_checkpoint_fix
Reviewer Review Type Date Requested Status
Stephan Kramer Pending
Review via email:

Description of the change

When an element is a string diamond expects an attribute with a hint about the number of lines.

This merge modifies spud to add this attribute when writing out the options to file.

This fixes checkpoints (where new initial condition elements are used containing file names) so that they aren't incomplete when opened first in diamond.

To post a comment you must log in.
Revision history for this message
Stephan Kramer (s-kramer) wrote :

Sounds eminently sensible. Why does it write lines="20" for any n/o lines>1 though?

Revision history for this message
Cian Wilson (cwilson) wrote :

Yeah, not really sure what to do there. The schema has either lines=1 or lines=20 depending on the type of string it is - basically filenames are assumed 1 and code is assumed 20. So I was just trying to emulate that.

The odd thing about this change is that I think string options that were read in with a lines attribute will be overwritten by this even though they know the right answer as they will have parsed the lines attribute at the start.

An alternative fix that I thought of was checking if lines was set when set_option is called on a string and setting an attribute then (this option is complicated a bit by the way spud stores attributes - basically as string elements with no children).

Otherwise I could try to see when we write a file whether the lines attribute has already been set and only set it if it hasn't.

Revision history for this message
Cian Wilson (cwilson) wrote :

Couldn't think of a robust way of doing this within spud so decided to go another way with this, fixing it in my code rather than in spud by addressing the attribute to the internal spud option path (__value/lines). Not perfect but without a major overhaul of spud it'll do.

Thanks for taking a look Stephan.

Unmerged revisions

538. By Cian Wilson

Fix string_type elements to have a lines attribute.

This fixes checkpoints so that they aren't missing this attribute when first opened in diamond.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/spud.cpp'
2--- src/spud.cpp 2015-08-12 17:43:53 +0000
3+++ src/spud.cpp 2015-11-21 04:58:37 +0000
4@@ -1706,6 +1706,7 @@
5 ele->LinkEndChild(data_ele);
7 for(deque< pair<string, Option*> >::const_iterator iter = children.begin();iter != children.end();iter++){
8+ string str = iter->second->data_as_string();
9 if(iter->second->is_attribute){
10 // Add attribute
11 ele->SetAttribute(iter->second->node_name, iter->second->data_as_string());
12@@ -1724,6 +1725,7 @@
13 break;
14 case(SPUD_STRING):
15 child_ele->SetValue("string_value");
16+ child_ele->SetAttribute("lines", (std::count(str.begin(), str.end(), '\n') > 0 ? "20" : "1"));
17 break;
18 default:
19 cerr << "SPUD ERROR: Invalid option type" << endl;


People subscribed via source and target branches