Small cleanups.

git-svn-id: svn://localhost/ardour2/branches/3.0@7686 d708f5d6-7413-0410-9779-e7cbd77b26cf

4 files changed, 62 insertions, 91 deletions

diff --git a/libs/ardour/ardour/playlist.h b/libs/ardour/ardour/playlist.h
index 43cb80418a..624f0a3da1 100644
--- a/libs/ardour/ardour/playlist.h
+++ b/libs/ardour/ardour/playlist.h
@@ -65,7 +65,7 @@ class RegionListProperty : public PBD::SequenceProperty<std::list<boost::shared_
 
 	RegionListProperty* clone () const;
 
-        boost::shared_ptr<Region> lookup_id (const PBD::ID& id);
+        boost::shared_ptr<Region> lookup_id (const PBD::ID& id) const;
 
   private:
 	RegionListProperty* create () const;
@@ -92,7 +92,7 @@ public:
         void clear_owned_changes ();
         void rdiff (std::vector<PBD::StatefulDiffCommand*>&) const;
 
-	boost::shared_ptr<Region> region_by_id (const PBD::ID&);
+	boost::shared_ptr<Region> region_by_id (const PBD::ID&) const;
 
 	void set_region_ownership ();
 
diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc
index 2c4fa5382b..93cfbab1fc 100644
--- a/libs/ardour/playlist.cc
+++ b/libs/ardour/playlist.cc
@@ -113,7 +113,7 @@ RegionListProperty::RegionListProperty (Playlist& pl)
 }
 
 boost::shared_ptr<Region>
-RegionListProperty::lookup_id (const ID& id)
+RegionListProperty::lookup_id (const ID& id) const
 {
         boost::shared_ptr<Region> ret =  _playlist.region_by_id (id);
         
@@ -2673,7 +2673,7 @@ Playlist::find_region (const ID& id) const
 }
 
 boost::shared_ptr<Region>
-Playlist::region_by_id (const ID& id)
+Playlist::region_by_id (const ID& id) const
 {
 	/* searches all regions ever added to this playlist */
 
diff --git a/libs/pbd/pbd/properties.h b/libs/pbd/pbd/properties.h
index aeb0c3e069..bd4846dc7e 100644
--- a/libs/pbd/pbd/properties.h
+++ b/libs/pbd/pbd/properties.h
@@ -122,7 +122,6 @@ public:
 	
 	void apply_changes (PropertyBase const * p) {
 		T v = dynamic_cast<const PropertyTemplate<T>* > (p)->val ();
-		std::cout << "Apply changes: " << v << " cf " << _current << "\n";
 		if (v != _current) {
 			set (v);
 		}
@@ -132,9 +131,14 @@ public:
 		T const tmp = _current;
 		_current = _old;
 		_old = tmp;
-		std::cout << "Inverted to " << _old << " -> " << _current << "\n";
 	}
 
+        void get_changes_as_properties (PropertyList& changes, Command *) const {
+                if (this->_have_old) {
+			changes.add (clone ());
+                }
+        }
+
 protected:
         /** Constructs a PropertyTemplate with a default
             value for _old and _current.
@@ -198,12 +202,6 @@ public:
 		return new Property<T> (*this);
 	}
 	
-        void get_changes_as_properties (PropertyList& changes, Command *) const {
-                if (this->_have_old) {
-			changes.add (new Property<T> (*this));
-                }
-        }
-
         Property<T>* clone_from_xml (const XMLNode& node) const {
 		XMLNodeList const & children = node.children ();
 		XMLNodeList::const_iterator i = children.begin();
@@ -275,12 +273,6 @@ public:
 		return new Property<std::string> (*this);
 	}
 	
-        void get_changes_as_properties (PropertyList& changes, Command* /*ignored*/) const {
-                if (this->_have_old) {
-			changes.add (new Property<std::string> (*this));
-		}
-        }
-
 	std::string & operator=(std::string const& v) {
 		this->set (v);
 		return this->_current;
diff --git a/libs/pbd/pbd/sequence_property.h b/libs/pbd/pbd/sequence_property.h
index 7fdc964397..5b37d7a0fc 100644
--- a/libs/pbd/pbd/sequence_property.h
+++ b/libs/pbd/pbd/sequence_property.h
@@ -80,7 +80,7 @@ class SequenceProperty : public PropertyBase
 	SequenceProperty (PropertyID id, const boost::function<void(const ChangeRecord&)>& update)
                 : PropertyBase (id), _update_callback (update) {}
 
-	virtual typename Container::value_type lookup_id (const PBD::ID&) = 0;
+	virtual typename Container::value_type lookup_id (const PBD::ID&) const = 0;
 
         void invert () {
 		_changes.removed.swap (_changes.added);
@@ -147,43 +147,63 @@ class SequenceProperty : public PropertyBase
 	}
 
 	void get_changes_as_properties (PBD::PropertyList& changes, Command* cmd) const {
-		if (changed ()) {
-			SequenceProperty<Container>* a = copy_for_history ();
-			changes.add (a);
-
-			if (cmd) {
-				/* whenever one of the items emits DropReferences, make sure
-				   that the Destructible we've been told to notify hears about
-				   it. the Destructible is likely to be the Command being built
-				   with this diff().
-				*/
+		if (!changed ()) {
+			return;
+		}
+		
+		/* Create a property with just the changes and not the actual values */
+		SequenceProperty<Container>* a = create ();
+		a->_changes = _changes;
+		changes.add (a);
+		
+		if (cmd) {
+			/* whenever one of the items emits DropReferences, make sure
+			   that the Destructible we've been told to notify hears about
+			   it. the Destructible is likely to be the Command being built
+			   with this diff().
+			*/
                         
-				for (typename ChangeContainer::iterator i = a->changes().added.begin(); i != a->changes().added.end(); ++i) {
-					(*i)->DropReferences.connect_same_thread (*cmd, boost::bind (&Destructible::drop_references, cmd));
-				}
-                        }
+			for (typename ChangeContainer::iterator i = a->changes().added.begin(); i != a->changes().added.end(); ++i) {
+				(*i)->DropReferences.connect_same_thread (*cmd, boost::bind (&Destructible::drop_references, cmd));
+			}
 		}
         }
 
 	SequenceProperty<Container>* clone_from_xml (XMLNode const & node) const {
 
 		XMLNodeList const children = node.children ();
+
+		/* find the node for this property name */
+		
+		std::string const c = capitalize (property_name ());
+		XMLNodeList::const_iterator i = children.begin();
+		while (i != children.end() && (*i)->name() != c) {
+			++i;
+		}
+
+		if (i == children.end()) {
+			return 0;
+		}
+
+		/* create a property with the changes */
 		
-		for (XMLNodeList::const_iterator i = children.begin(); i != children.end(); ++i) {
-
-			if ((*i)->name() == capitalize (property_name())) {
-				
-				SequenceProperty<Container>* p = create ();
-				
-				if (p->set_changes (**i)) {
-					return p;
-				} else {
-					delete p;
-				}
+		SequenceProperty<Container>* p = create ();
+
+		XMLNodeList const & grandchildren = (*i)->children ();
+		for (XMLNodeList::const_iterator j = grandchildren.begin(); j != grandchildren.end(); ++j) {
+			XMLProperty const * prop = (*j)->property ("id");
+			assert (prop);
+			PBD::ID id (prop->value ());
+			typename Container::value_type v = lookup_id (id);
+			assert (v);
+			if ((*j)->name() == "Add") {
+				p->_changes.added.insert (v);
+			} else if ((*j)->name() == "Remove") {
+				p->_changes.removed.insert (v);
 			}
-                }
+		}
 
-		return 0;
+		return p;
         }
 
 	void clear_owned_changes () {
@@ -310,54 +330,13 @@ class SequenceProperty : public PropertyBase
         
         const ChangeRecord& changes () const { return _changes; }
 
-  protected:
-	Container _val;
-	ChangeRecord _changes;
+protected:
+	Container _val; ///< our actual container of things
+	ChangeRecord _changes; ///< changes to the container (adds/removes) that have happened since clear_changes() was last called
 	boost::function<void(const ChangeRecord&)> _update_callback;
 
-        /** Load serialized change history.
-         * @return true if loading succeeded, false otherwise
-         */
-
-	bool set_changes (XMLNode const & history_node) {
-
-                const XMLNodeList& children (history_node.children());
-
-                for (XMLNodeList::const_iterator i = children.begin(); i != children.end(); ++i) {
-                        const XMLProperty* prop = (*i)->property ("id");
-                        if (prop) {
-                                PBD::ID id (prop->value());
-                                typename Container::value_type v = lookup_id (id);
-                                if (!v) {
-                                        std::cerr << "No such item, ID = " << id.to_s() << " (from " << prop->value() << ")\n";
-                                        return false;
-                                }
-                                if ((*i)->name() == "Add") {
-                                        _changes.added.insert (v);
-                                } else if ((*i)->name() == "Remove") {
-                                        _changes.removed.insert (v);
-                                }
-                        }
-                }
-
-                return true;
-        }
-
-private:
+private:	
 	virtual SequenceProperty<Container>* create () const = 0;
-
-        /* create a copy of this ListSequenceProperty that only
-           has what is needed for use in a history list command. This
-           means that it won't contain the actual item list but
-           will have the added/removed list.
-        */
-	
-	SequenceProperty<Container>* copy_for_history () const {
-		SequenceProperty<Container>* copy = create ();
-		/* this is all we need */
-		copy->_changes = _changes;
-		return copy;
-	}
 };
 
 }