[Clam-devel] Copy & Paste processings on canvas patch

Pau Arumí Albó parumi at iua.upf.edu
Tue Jun 10 02:03:41 PDT 2008


El dt 10 de 06 de 2008 a les 03:31 +0200, en/na David García Garzón va
escriure:
> Pau, i just commited the patch previous to the pos and sizes because it 
> modifies the network XML and further changes might be done with care because 
> they might break stored networks. We should be sure that the format is the 
> one to stay for a while.

Yes we'll need to be sure about the new format.
And I expect that backwards compatibility won't be a problem. Natanael,
warn us if you think so.

BTW, I notice that the size/position extension only effects the
copy-paste and not the network saving. Is this expected?


> The point is that the proposed patch stores the position and the size as 
> string:
> boxPos="20,25" boxSize="400,20"
> Is that what we want?
> not x='20' y='25 height='20' widht='400'
> or even position='20,25' size='400,20'?

My vote is clearly for the last one (away with boxXxx). 
And using integers (or floats) all the way till the very last step is
safer, and more coherent with the existing format. 
However, if changing this is too complex we might leave it as is now.
Natanael?



> I also guess that the procAttribMap to store such attributes is a too generic 
> tool that can become subject of abuse if it is in the api. We are expected to 
> have some kind of network configuration/attributes and i don't know if this 
> is missleading when we actually want pos and size. Just a feeling not a 
> conviction. In this part we are more in Pau's zone.

Well in the patch I'm looking now, if I see it correclty, the name is
always "BoxesAttributes". It sound ok, though not perfect, to me. Maybe
"ProcessingAppearance"? 

> Natanael, you should also review the naming conventions in the new functions: 
> Some local variables starting with upper case and some CripAbrvOnNams. 
> The worst thing on abreviating words is that you never remember which letters 
> you removed. Modern editors have completition. You can also use the context, 
> if the only ones in your context to have positions and sizes are processings  
> you can remove the Proc and call them prositions and sizes.

And also: ifHasSelectionAndContains(...) ->
HasSelectioniAndContains(...)

Great work!
Pau





More information about the clam-devel mailing list