rationalize split deletiton, make it work right.

git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@1242 57a11ea4-9604-0410-9ed3-97b8803252fd

src/engine/Account.c

@@ -110,6 +110,7 @@ xaccFreeAccount( Account *acc )
 {
   int i=0;
   Split *s;
+  Transaction *t;
 
   if (NULL == acc) return;
     
@@ -129,12 +130,17 @@ xaccFreeAccount( Account *acc )
     s->acc = NULL;
   }
 
-  /* search for orphaned transactions, and delete them */
+  /* destroy all of the splits. The xaccCommitEdit() call
+   * will automatically clean up orphaned transactions.
+   */
   acc->open |= ACC_BEING_DESTROYED;
   acc->open |= ACC_DEFER_REBALANCE;
   for (i=0; i<acc->numSplits; i++) {
     s = acc->splits[i];
+    t = s->parent;
+    xaccTransBeginEdit (t, 1);
     xaccSplitDestroy (s);
+    xaccTransCommitEdit (t);
   }
 
   /* free up array of split pointers */

src/engine/Transaction.c

@@ -677,6 +677,37 @@ xaccTransCommitEdit (Transaction *trans)
    if (!trans) return;
    CHECK_OPEN (trans);
 
+   /* At this point, we check to see if we have a valid transaction.
+    * As a result of editing, we could end up with a transaction that
+    * has no splits in it, in which case we delete the transaction and
+    * return.  Alternately the transaction may have only one split in 
+    * it, in which case ... that's OK if and only if the split has no 
+    * value (i.e. is only recording a price).  Otherwise, a single
+    * split with a value can't possibly balance, thus violating the 
+    * rules of double-entry, and that's way bogus. So delete 
+    * the split, delete the transaction.  I suppose we could 
+    * generate an error or something like that at this point,
+    * to let the user know that we blew away a split.
+    */
+
+   split = trans->splits[0];
+   if (!split ||
+       ((NULL == trans->splits[1]) && (!(DEQ(0.0, split->damount)))))
+   {
+      /* Make a log in the journal before destruction.  */
+      xaccTransWriteLog (trans, 'D');
+      if (split) {
+         acc = split->acc;
+         MARK_SPLIT (split);
+         xaccAccountRemoveSplit (acc, split);
+         xaccAccountRecomputeBalance (acc);
+         xaccFreeSplit (split);
+         trans->splits[0] = NULL;
+      }
+      xaccFreeTransaction (trans);
+      return;
+   }
+
    trans->open &= ~DEFER_REBALANCE;
    xaccTransRebalance (trans);
 
@@ -749,8 +780,7 @@ xaccSplitDestroy (Split *split)
    trans = split->parent;
    assert (trans);
    assert (trans->splits);
-   // temp hack alert -- get rid of annoying error messages
-   // CHECK_OPEN (trans);
+   CHECK_OPEN (trans);
 
    numsplits = 0;
    s = trans->splits[0];
@@ -765,42 +795,20 @@ xaccSplitDestroy (Split *split)
    /* If the account has three or more splits, 
     * merely unlink & free the split. 
     *
-    * Or if the account has only two splits, and the 
-    * split to be destroyed is a price split (i.e.
-    * has a damount value of zero), then
-    * merely unlink & free the split. 
+    * Or if the account has only two splits, 
+    * then this destroy will leave only one split.
+    * Don't rebalance, as this will goof up the
+    * value of teh remaining split.
     */
-   if ((2 < numsplits) ||
-       ((2 == numsplits) && (DEQ(0.0, split->damount))))
-   {
-      MARK_SPLIT (split);
-      xaccTransRemoveSplit (trans, split);
-      acc = split->acc;
-      xaccAccountRemoveSplit (acc, split);
-      xaccAccountRecomputeBalance (acc);
-      xaccFreeSplit (split);
+   MARK_SPLIT (split);
+   xaccTransRemoveSplit (trans, split);
+   acc = split->acc;
+   xaccAccountRemoveSplit (acc, split);
+   xaccAccountRecomputeBalance (acc);
+   xaccFreeSplit (split);
+
+   if (2 < numsplits) {
       xaccSplitRebalance (trans->splits[0]);
-   } else {
-      /* if the transaction has only two splits,
-       * remove both of them, and them destroy the 
-       * transaction. Make a log in the journal before 
-       * destruction.
-       */
-      xaccTransWriteLog (trans, 'D');
-      s = trans->splits[0];
-      acc = s->acc;
-      MARK_SPLIT (s);
-      xaccAccountRemoveSplit (acc, s);
-      xaccAccountRecomputeBalance (acc);
-
-      s = trans->splits[1];
-      if (s) {
-         acc = s->acc;
-         MARK_SPLIT (s);
-         xaccAccountRemoveSplit (acc, s);
-         xaccAccountRecomputeBalance (acc);
-         xaccFreeTransaction (trans);
-      }
    }
 }
 

src/engine/Transaction.h

@@ -72,9 +72,13 @@ void          xaccInitTransaction (Transaction *);/* clears a trans struct */
  */
 void          xaccTransDestroy (Transaction *);
 
-/* The xaccTransBegineEdit() ... 
+/* The xaccTransBeginEdit() ... 
  *    If the defer flag is set, then automated balancing
  *    is defered until the commit ...
+ *
+ * The xaccTransCommitEdit() routine may result in the deletion of the
+ * transaction, if the transaction is "empty" (has no splits, or
+ * has a single split in it whose value is non-zero.)
  */
 void          xaccTransBeginEdit (Transaction *, int defer);
 void          xaccTransCommitEdit (Transaction *);
@@ -115,10 +119,13 @@ void          xaccTransAppendSplit (Transaction *, Split *);
  *    leaving the accounting structure out-of-balance or otherwise
  *    inconsistent.
  *
- *    If the parent transaction of the split has three or more splits
- *    in it, then only this one split is unlinked. If the parent
- *    transaction has only two splits in it (and thus, this is one of
- *    them), then both splits and the transaction are destroyed.
+ *    If the deletion of the split leaves the transaction "empty",
+ *    then the transaction will be marked for deletion.  (It will
+ *    not be deleted until the xaccTransCommitEdit() routine is called.)
+ *    The transaction is considered "empty" if it has no splits in it, 
+ *    or it has only one split left, and that split is not a price split
+ *    (i.e. has a non-zero value).  Transactions with only one split in 
+ *    them are valid if and only if the value of that split is zero.
  */
 void          xaccSplitDestroy (Split *);