better way to deal with undo+id collisions

diff --git a/src/document-private.h b/src/document-private.h
index c89948290974a14c155a182e8ec401faf1471c9f..1c03a8025d44ec3a9349bc14a648aefc82c9d444 100644 (file)
--- a/src/document-private.h
+++ b/src/document-private.h
@@ -70,6 +70,8 @@ struct SPDocumentPrivate {
 
        // XXX only for testing!
        Inkscape::ConsoleOutputUndoObserver console_output_undo_observer;
+
+       bool seeking;
 };
 
 #endif

diff --git a/src/document-undo.cpp b/src/document-undo.cpp
index c7da41080bd8d3593786aec4b1642c986109b351..0462ff374a175fccd94e959c4958d1cda9e82a4c 100644 (file)
--- a/src/document-undo.cpp
+++ b/src/document-undo.cpp
@@ -7,6 +7,7 @@
  *   Lauris Kaplinski <lauris@kaplinski.com>
  *   MenTaLguY <mental@rydia.net>
  *
+ * Copyright (C) 2007  MenTaLguY <mental@rydia.net>
  * Copyright (C) 1999-2003 authors
  * Copyright (C) 2001-2002 Ximian, Inc.
  *
@@ -222,6 +223,7 @@ sp_document_undo (SPDocument *doc)
        g_assert (doc->priv->sensitive);
 
        doc->priv->sensitive = FALSE;
+        doc->priv->seeking = true;
 
        doc->actionkey = NULL;
 
@@ -244,6 +246,7 @@ sp_document_undo (SPDocument *doc)
        sp_repr_begin_transaction (doc->rdoc);
 
        doc->priv->sensitive = TRUE;
+        doc->priv->seeking = false;
 
        if (ret)
                inkscape_external_change();
@@ -266,6 +269,7 @@ sp_document_redo (SPDocument *doc)
        g_assert (doc->priv->sensitive);
 
        doc->priv->sensitive = FALSE;
+        doc->priv->seeking = true;
 
        doc->actionkey = NULL;
 
@@ -288,6 +292,7 @@ sp_document_redo (SPDocument *doc)
        sp_repr_begin_transaction (doc->rdoc);
 
        doc->priv->sensitive = TRUE;
+        doc->priv->seeking = false;
 
        if (ret)
                inkscape_external_change();

diff --git a/src/document.cpp b/src/document.cpp
index d77d61d0984862dd4c3ff070da6de6a0bf4bc309..c6dc66a514cd5675f5eaaf1831c561a648600f3e 100644 (file)
--- a/src/document.cpp
+++ b/src/document.cpp
@@ -106,6 +106,7 @@ SPDocument::SPDocument() {
     p->history_size = 0;
     p->undo = NULL;
     p->redo = NULL;
+    p->seeking = false;
 
     priv = p;
 
@@ -1155,6 +1156,10 @@ vacuum_document(SPDocument *document)
     return start - newend;
 }
 
+bool SPDocument::isSeeking() const {
+    return priv->seeking;
+}
+
 
 /*
   Local Variables:

diff --git a/src/document.h b/src/document.h
index 12848aa68f469442a75d50be0448ed805d031588..6e4bd8d6cf6599d117293e5adc53e7ff4a91eeb6 100644 (file)
--- a/src/document.h
+++ b/src/document.h
@@ -117,6 +117,9 @@ struct SPDocument : public Inkscape::GC::Managed<>,
 
         bool _updateDocument();
 
+       /// Are we currently in a transition between two "known good" states of the document?
+       bool isSeeking() const;
+
 private:
        SPDocument(SPDocument const &); // no copy
        void operator=(SPDocument const &); // no assign

diff --git a/src/sp-object.cpp b/src/sp-object.cpp
index 03dc3ec28cd822d972c523ccf82c029f8f1501c3..5dbc4101e7bfb3a392fa4132ab1f1676175f5c41 100644 (file)
--- a/src/sp-object.cpp
+++ b/src/sp-object.cpp
@@ -821,20 +821,26 @@ sp_object_invoke_build(SPObject *object, SPDocument *document, Inkscape::XML::No
         object->document->bindObjectToRepr(object->repr, object);
 
         if (Inkscape::XML::id_permitted(object->repr)) {
-            /* If we are not cloned, force unique id */
+            /* If we are not cloned, and not seeking, force unique id */
             gchar const *id = object->repr->attribute("id");
-            gchar *realid = sp_object_get_unique_id(object, id);
-            g_assert(realid != NULL);
-
-            object->document->bindObjectToId(realid, object);
-            object->id = realid;
-
-            /* Redefine ID, if required */
-            if ((id == NULL) || (strcmp(id, realid) != 0)) {
-                bool saved = sp_document_get_undo_sensitive(document);
-                sp_document_set_undo_sensitive(document, false);
-                object->repr->setAttribute("id", realid);
-                sp_document_set_undo_sensitive(document, saved);
+            if (!document->isSeeking()) {
+                gchar *realid = sp_object_get_unique_id(object, id);
+                g_assert(realid != NULL);
+
+                object->document->bindObjectToId(realid, object);
+                object->id = realid;
+
+                /* Redefine ID, if required */
+                if ((id == NULL) || (strcmp(id, realid) != 0)) {
+                    object->repr->setAttribute("id", realid);
+                }
+            } else if (id) {
+                // bind if id, but no conflict -- otherwise, we can expect
+                // a subsequent setting of the id attribute
+                if (!object->document->getObjectById(id)) {
+                    object->document->bindObjectToId(id, object);
+                    object->id = g_strdup(id);
+                }
             }
         }
     } else {
@@ -945,16 +951,23 @@ sp_object_private_set(SPObject *object, unsigned int key, gchar const *value)
                 SPDocument *document=object->document;
                 SPObject *conflict=NULL;
 
-                if (value) {
-                    conflict = document->getObjectById((char const *)value);
+                gchar const *new_id = value;
+
+                if (new_id) {
+                    conflict = document->getObjectById((char const *)new_id);
                 }
+
                 if ( conflict && conflict != object ) {
-                    sp_object_ref(conflict, NULL);
-                    // give the conflicting object a new ID
-                    gchar *new_conflict_id = sp_object_get_unique_id(conflict, NULL);
-                    SP_OBJECT_REPR(conflict)->setAttribute("id", new_conflict_id);
-                    g_free(new_conflict_id);
-                    sp_object_unref(conflict, NULL);
+                    if (!document->isSeeking()) {
+                        sp_object_ref(conflict, NULL);
+                        // give the conflicting object a new ID
+                        gchar *new_conflict_id = sp_object_get_unique_id(conflict, NULL);
+                        SP_OBJECT_REPR(conflict)->setAttribute("id", new_conflict_id);
+                        g_free(new_conflict_id);
+                        sp_object_unref(conflict, NULL);
+                    } else {
+                        new_id = NULL;
+                    }
                 }
 
                 if (object->id) {
@@ -962,8 +975,8 @@ sp_object_private_set(SPObject *object, unsigned int key, gchar const *value)
                     g_free(object->id);
                 }
 
-                if (value) {
-                    object->id = g_strdup((char const*)value);
+                if (new_id) {
+                    object->id = g_strdup((char const*)new_id);
                     document->bindObjectToId(object->id, object);
                 } else {
                     object->id = NULL;