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

[Pau Arumí Albó]

El dj 05 de 06 de 2008 a les 12:37 +0200, en/na David García Garzón va
escriure:
> On Dijous 05 Juny 2008, Pau Arumí Albó wrote:
> > El dj 05 de 06 de 2008 a les 06:15 -0300, en/na Natanael Olaiz va
> >
> > escriure:
> > > El 06/04/2008 06:17 AM, Pau Arumí Albó escribió:
> > > > El dc 04 de 06 de 2008 a les 03:58 -0300, en/na Natanael Olaiz va
> > > >
> > > > escriure:
> > > >> El 06/03/2008 03:16 PM, Pau Arumí escribió:
> > > >>> On dt, 2008-06-03 at 04:48 -0300, Natanael Olaiz wrote:
> > > >>>> El 06/02/2008 08:14 AM, David García Garzón escribió:
> > > >>>>> On Dilluns 02 Juny 2008, Pau Arumí Albó wrote:
> > > >>>>>> El dl 02 de 06 de 2008 a les 06:19 -0300, en/na Natanael Olaiz va
> > > >>>>>>
> > > >>>>>> escriure:
> > > >>>>>>> Finally!
> > > >>>>>>> I need to clean up the code, optimize some rutines, and it
> > > >>>>>>> doesn't save the positions. But it works. :-)
> > > >>>>>>
> > > >>>>>> Wow, this is big step forwards! :-)
> > > >>>>>>
> > > >>>>>> Some questions mainly for Natanael and David,
> > > >>>>>>
> > > >>>>>> Have you evaluated if the onPasteProcessingsFromClipboard() could
> > > >>>>>> reuse the normal xml network loading function? (maybe would need
> > > >>>>>> adaptation) ? That would reduce a lot of code (and duplication)
> > > >>>>>
> > > >>>>> I think so but some adaptation should be done to the
> > > >>>>> NetworkAdapters to store just the selected processings and related
> > > >>>>> connections.
> > > >>>>
> > > >>>> OK, I'll see that. It could be also good to start thinking about
> > > >>>> subnetworks paths.
> > > >>>>
> > > >>>>>> I think that this feature (and the future sub-networks) is asking
> > > >>>>>> for embedding processing-boxes positions in the clamnetwork xml
> > > >>>>>> (instead of keeping a separate binary .pos file). Getting rid of
> > > >>>>>> .pos files was a TODO actually. Do you agree?
> > > >>>>>
> > > >>>>> I fully agree.
> > > >>>>
> > > >>>> Me too. Another task to add. :-)
> > > >>>
> > > >>> After some chat with David about this, I think it would be good to
> > > >>> start with 1) reuse the network loading/saving code, before starting
> > > >>> 2) adding positions to the xml. This is because having 2) will make
> > > >>> the load/save more difficult, as needs transferring data from
> > > >>> canvas/procboxes to the network, and should be better approached as a
> > > >>> posterior refactoring.
> > > >>>
> > > >>> Don't hesitate to ask if find problems
> > > >>
> > > >> Should I store the positions & sizes within the Network root path, or
> > > >> on a separate one (let's call it "NetworkCanvas", for instance)? In
> > > >> the second case I will need to make two calls to dump/restore methods
> > > >> (one within the network -as right now-, and the other within the
> > > >> canvas), but that keeps the data separated and doesn't need to
> > > >> transfer any new data to the network.
> > > >>
> > > >> Is my question clear enough? What do you prefer?
> > > >
> > > > It makes sense to me but I'm not sure about the details -- specially
> > > > who and how to close the xml doc if we have two dump calls?
> > > > One possible solution that comes to my mind is adding a new string
> > > > argument to the Dump mothod (or function, don't remember) containing an
> > > > "extra root" with the positions and sizes.
> > > >
> > > > However, this is 2) and, IMO, 1) should be done before.
> > >
> > > Sorry to insist.... but thinking about it, I'm wondering how to reuse
> > > the code to save selections without get the same problem of positions.
> > > The selections are attributes of ProcessingBoxes, just like positions!
> > > So, I can pass the list of selected processings to the Network, but I
> > > think doing that is maybe more weired than just start doing the
> > > integration in a higher level, in ClamNetworkCanvas, deriving it from
> > > Components, redefining StoreOn and LoadFrom, and make the
> > > ProcessingBoxes correspondents definition adaptions...
> >
> > Well yes I see it's not an easy problem.
> > The main problem I see implementing Store and Load methods in
> > NetworkCanvas is that will duplicate the code in Network::StoreOn.
> > And we need this functinality in Network for the NE-less use cases.
> > What about adding optional decorations (in the design pattern sense) in
> > the Processing in the network containing the Canvas data: position,
> > size, isSelected (so that you can store only the selected processings in
> > a given string).
> 
> I think that the decorator already exists: it is the 
> ProcessingDefinitionAdapter it is a matter of adding the position information 
> besides the name.
> 
> > Then when the Canvas store would consist on first pushing the
> > canvas-related data into the network and then call the normal Store (or
> > Dump). And similarly for Loading.
> >
> > However at this point I'd like to have David's input, he is the expert
> > on NE user-interface.
> 
> 
> Well i think it is very important to keep the network, canvas agnostic, as 
> well as keeping the NetworkCanvas clam agnostic. But as we are introducing 
> the positions and the selections as part of the network itself i propose to 
> add such an information to the network itself, not as part of the processings 
> but as network optional attributes. That is duplicating the information but 
> just during loading and storing. The key to eliminate a duplication hell is 
> declaring such information in the Network is not reliable, is the canvas who 
> set it before loading and recovers it after saving but clears it afterwards. 
> So there is no temptation on using it in a different place than load/store.
> 
> So how to implement that? For selections adding the network a std::set 
> attribute of selected processing names. It has just effect on storing and 
> only if it is not empty. If it is not empty the network should check if the 
> name is in the set before storin it. On storing a selection and just on 
> storing a selection call Network::UpdateSelections(list or other container), 
> do the store and then clear them Network::ClearSelections.
> The Network::StoreOn will only store the processing whose name is in the set, 
> or all of them if the set is empty. I think that this one would be the easier 
> one and the first to be addressed.
> 
> About the positions and sizes, i would use the same strategy. On load/store 
> push or extract such information into the network being reliable just on load 
> and store time, clearing it afterwards to be sure we don't rely on it after 
> load/store. Such information should be transfered if present to the 
> ProcessingDefinitionAdapter to xml them. Not that clear about the structure 
> to store size and position but let's address first selections and then, let's 
> see what about sizes and positions.

I agree with the approach.
Now let's see if Natanel can defeat the evil details :-) 

As David says, let's first address the selections part, since this will
define the  PBox-Canvas-Network communication while maintaining the
changeset small.

P