Forgotten patch: qofid.diff

Chris Shoemaker c.shoemaker at cox.net
Wed Oct 26 23:36:02 EDT 2005


[WARNING, my response to Neil might get polemic.  If you've already
exceeded your daily tolerance for my suggestions, PLEASE JUST SKIP to
the patch.  I'm serious, really.]

On Thu, Oct 27, 2005 at 12:51:54AM +0100, Neil Williams wrote:
> On Wednesday 26 October 2005 11:27 pm, Chris Shoemaker wrote:
> > Wow!  You had me at double iteration!  :)
> 
> :-))
> 
> As clear an example as I can imagine of how sharing code BEFORE it is "final 
> quality" is a wise choice for free software development! (Sorry it was your 
> code that got used for the illustration, Chris!)  ;-)
> 
> I should have been able to see your early code and you should be able to see 
> cashutil. Let's not let other new developers make the same mistakes next 
> time? All this work for a lack of documentation, easy to update webpages and 
> a little consensus on code management conventions.
> 
> We need quick and open ways to publish *early* code - pseudo code if you will. 
> Designs, plans, ideas. Rough hacks, concepts, logic. Code that is certain to 
> break the sacrosanct CoreBuild. Branches are part of the answer but, as Derek 
> pointed out, technology isn't the answer to sociological problems. 

I know nobody liked my ideas about branching, but maybe I'm just using
the term differently than others.  There's a interesting case-study
right here.  In my view, *every* time you're writing code, you're
implicitly branching whether you realize it or not.  For example, the
code was at point (A) when I last refreshed by budget patches (B) and
mailed them.  David continued to improve general GUI code (C).  Then
David merged (B) to (C), fixing the conflicts and also improving on my
code, producing (D).  Now, I've improved (B) from Neil's suggestion,
producing (E).

      /=B==E 
     /   \
   -A     \
     \==C==D

The goal now is to merge E->D.  I think the SCM question isn't so much
about branching as it is about merging.  Any merging approach based on
versioned trees will involve repeating the work done to merge B->C in
order to merge E->D.  Of course, what we *want* to do is not really
merge E->D, but only merge the delta from B to E into D.  (BTW, that
delta is exactly what I'm attaching.)  IOW, we're merging changesets,
whether we do it manually or our tools do it for us.  (Of course, they
can only do that if they *have* B delta E and *know* that B was merged
into C.)

The requirements are the same for the manual operation.  In this case,
we're not so bad off, because we do have B -- it was sent to the list.
And the only reason I'm sending B delta E is because I do know that B
was merged into C.

Now, I've seen this pattern probably 20 times (not with gnucash) and
it can get pretty hairy.  The only time this process doesn't really
suck is when I know somebody's been merging my code, and exactly what
state they were merging from, (like now).

IMO, this is a PITA, because I *REALLY* DON'T WANT TO CARE who's been
doing whatever with whatever I've written.  I'm *sharing* my code for
a reason: I want other people to benefit from it.  That benefit is
severely diminished if it's a pain for other people to independently
improve my code and still retain my further improvements, too, or if I
have to keep tabs on everything that happens to every revision of my
code just to make it *not* a pain to incorporate further improvements.

To be frank, I'm convinced that for me to continue in that code
development pattern is irresponsible and wasteful.

> The core 
> advance has to be that we leave our paranoia behind, kill the sacred cow of 
> "TheCoreBuild" and accept that you can't make an omelette without breaking 
> eggs. CVS is NOT guaranteed to always build, there's no rule that says so and 
> we mislead ourselves and prospective new developers if we pursue such a goal 
> *to the exclusion of all else*. Fine, have a HEAD branch that is the current 
> release and maybe a main development branch but sometimes that build will 
> break. It's a necessary step. It's a GOOD sign, it is to be managed not 
> eradicated as if it was some strain of avian flu.

I actually don't have any problem with there being some branch
(whatever it's called) that has a very high standard for merging.
Now, I certainly don't want to be the one responsible for maintaining
it, but I'd sure have a healthy amount of respect for anyone who did.
OTOH, I'm really doing my best to make sure that that's not the de
facto standard for *sharing* code, which I think is just idiotic.

****

Just so I can keep all my rant in one mail ... This code sharing idea
is not some hypothetical.  I've got a fair chuck of register rewrite
code and Didier expressed an interest in collaborating.  We've seen
him put up and code so I'm inclined to take his offer seriously.  

Right now, my options are:

1) stream a bunch of patches to -patches that don't always compile,
are obviously incomplete and ugly, have dependencies higher than our
target release, probably break the build, and probably shouldn't be
applied by anyone not working on the register rewrite; and hope
Didier's tools are handy enough to autosuck patches from email (oooh
distributed development 1980's style)

or 2) take this into a back-alley and get some real collaboration
going, even though it'll be in relative isolation from the people who
have the most to offer in terms of assistance.

*sigh* Ok, I'm done.  :( Please include the word FLAME in the subject
of your response if I'll need my asbestos underwear.

-chris


-------------- next part --------------
Index: gnucash/src/engine/gnc-budget.c
===================================================================
--- gnucash.orig/src/engine/gnc-budget.c
+++ gnucash/src/engine/gnc-budget.c
@@ -335,16 +335,6 @@ gnc_budget_get_book(GncBudget* budget)
     return qof_instance_get_book(&budget->inst);
 }
 
-GList*
-gnc_book_get_budgets(QofBook* book)
-{
-    QofCollection *col;
-
-    g_return_val_if_fail(book, NULL);
-    col = qof_book_get_collection (book, GNC_ID_BUDGET);
-    return qof_collection_get_list(col);
-}
-
 GncBudget*
 gnc_budget_lookup (const GUID *guid, QofBook *book)
 {
@@ -356,17 +346,22 @@ gnc_budget_lookup (const GUID *guid, Qof
     return GNC_BUDGET(qof_collection_lookup_entity (col, guid));
 }
 
+static void just_get_one(QofEntity *ent, gpointer data)
+{
+    GncBudget **bgt = (GncBudget**)data;
+    if (bgt && !*bgt) *bgt = GNC_BUDGET(ent);
+}
+
 GncBudget*
 gnc_budget_get_default (QofBook *book)
 {
-    GList *list;
+    QofCollection *col;
     GncBudget *bgt = NULL;
 
     g_return_val_if_fail(book, NULL);
-    list = gnc_book_get_budgets(book);
-    if (g_list_length(list) > 0) {
-        bgt = GNC_BUDGET(list->data);  // Just get the first one.
-        g_list_free(list);
+    col = qof_book_get_collection(book, GNC_ID_BUDGET);
+    if (qof_collection_count(col) > 0) {
+        qof_collection_foreach(col, just_get_one, &bgt);
     }
     return bgt;
 }
Index: gnucash/src/gnome-utils/gnc-tree-model-budget.c
===================================================================
--- gnucash.orig/src/gnome-utils/gnc-tree-model-budget.c
+++ gnucash/src/gnome-utils/gnc-tree-model-budget.c
@@ -10,16 +10,13 @@
 #include "gnc-ui-util.h"
 
 /* Add the new budget object to the tree model.  */
-static void add_budget_to_model( gpointer data, gpointer user_data )
+static void add_budget_to_model(QofEntity* data, gpointer user_data )
 {
-    GncBudget* budget;
     GtkTreeIter iter;
-    GtkTreeModel* treeModel;
+    GncBudget* budget = GNC_BUDGET(data);
+    GtkTreeModel* treeModel = user_data;
 
-    budget = data;
     g_return_if_fail(GNC_IS_BUDGET(budget));
-    treeModel = user_data;
-
     g_return_if_fail(budget && treeModel);
 
     gtk_list_store_append (GTK_LIST_STORE(treeModel), &iter);
@@ -27,8 +24,7 @@ static void add_budget_to_model( gpointe
                         BUDGET_GUID_COLUMN, gnc_budget_get_guid(budget),
                         BUDGET_NAME_COLUMN, gnc_budget_get_name(budget),
                         BUDGET_DESCRIPTION_COLUMN,
-                        gnc_budget_get_description(budget),
-                        -1);
+                        gnc_budget_get_description(budget), -1);
 }
 
 /* CAS: Even though it works, something feels not-quite-right with
@@ -52,17 +48,15 @@ static void add_budget_to_model( gpointe
 GtkTreeModel *
 gnc_tree_model_budget_new(QofBook *book)
 {
-    GList *budgetList;
     GtkListStore* store;
 
     store = gtk_list_store_new (BUDGET_LIST_NUM_COLS,
                                 G_TYPE_POINTER,
                                 G_TYPE_STRING,
                                 G_TYPE_STRING);
-    budgetList = gnc_book_get_budgets(book);
-
-    g_list_foreach(budgetList, add_budget_to_model, GTK_TREE_MODEL(store));
-    g_list_free(budgetList);
+    
+    qof_collection_foreach(qof_book_get_collection(book, GNC_ID_BUDGET), 
+                           add_budget_to_model, GTK_TREE_MODEL(store));
 
     return GTK_TREE_MODEL(store);
 }
@@ -89,8 +83,8 @@ gnc_tree_view_budget_set_model(GtkTreeVi
 
 }
 
-GncBudget *gnc_tree_model_budget_get_budget(GtkTreeModel *tm,
-                                            GtkTreeIter *iter)
+GncBudget *
+gnc_tree_model_budget_get_budget(GtkTreeModel *tm, GtkTreeIter *iter)
 {
     GncBudget *bgt;
     GValue gv = { 0 };
@@ -104,9 +98,9 @@ GncBudget *gnc_tree_model_budget_get_bud
     return bgt;
 }
 
-void gnc_tree_model_budget_get_iter_for_budget(GtkTreeModel *tm,
-                                               GtkTreeIter *iter,
-                                               GncBudget *bgt)
+void 
+gnc_tree_model_budget_get_iter_for_budget(GtkTreeModel *tm, GtkTreeIter *iter,
+                                          GncBudget *bgt)
 {
     GValue gv = { 0 };
     const GUID *guid1;
@@ -115,9 +109,9 @@ void gnc_tree_model_budget_get_iter_for_
     g_return_if_fail(GNC_BUDGET(bgt));
 
     guid1 = gnc_budget_get_guid(bgt);
-    for ( gtk_tree_model_get_iter_first(tm, iter);
-          gtk_list_store_iter_is_valid(GTK_LIST_STORE(tm), iter);
-          gtk_tree_model_iter_next(tm, iter)) {
+    for (gtk_tree_model_get_iter_first(tm, iter);
+         gtk_list_store_iter_is_valid(GTK_LIST_STORE(tm), iter);
+         gtk_tree_model_iter_next(tm, iter)) {
 
         gtk_tree_model_get_value(tm, iter, BUDGET_GUID_COLUMN, &gv);
         guid2 = (GUID *) g_value_get_pointer(&gv);
Index: gnucash/src/gnome/gnc-plugin-budget.c
===================================================================
--- gnucash.orig/src/gnome/gnc-plugin-budget.c
+++ gnucash/src/gnome/gnc-plugin-budget.c
@@ -195,34 +195,38 @@ gnc_plugin_budget_cmd_new_budget (GtkAct
     gnc_main_window_open_page (data->window, page);
 }
 
+static void just_get_one(QofEntity *ent, gpointer data)
+{
+    GncBudget **bgt = (GncBudget**)data;
+    if (bgt && !*bgt) *bgt = GNC_BUDGET(ent);
+}
+
 /* If only one budget exists, open it; otherwise user selects one to open */
 static void
 gnc_plugin_budget_cmd_open_budget (GtkAction *action,
                                    GncMainWindowActionData *data)
 {
-    GList *budgets;
+    guint count;
     QofBook *book;
     GncBudget *bgt;
+    QofCollection *col;
     g_return_if_fail (data != NULL);
 
     book = gnc_get_current_book();
-    budgets = gnc_book_get_budgets(book);
-    if (budgets) {
-        if (g_list_length(budgets) == 1) {
-            bgt = GNC_BUDGET(budgets->data);
+    col = qof_book_get_collection(book, GNC_ID_BUDGET);
+    count = qof_collection_count(col);
+    if (count > 0) {
+        if (count == 1) {
+            qof_collection_foreach(col, just_get_one, &bgt);
         } else {
             bgt = gnc_budget_gui_select_budget(book);
         }
 
-        if (bgt)
-            gnc_main_window_open_page (
-                data->window, gnc_plugin_page_budget_new(bgt));
-
-    } else {
-        /* if no budgets exist yet, just open a new budget */
+        if (bgt) gnc_main_window_open_page(
+            data->window, gnc_plugin_page_budget_new(bgt));
+    } else { /* if no budgets exist yet, just open a new budget */
         gnc_plugin_budget_cmd_new_budget(action, data);
     }
-    g_list_free(budgets);
 }
 
 /************************************************************
Index: gnucash/src/backend/file/io-gncxml-v2.c
===================================================================
--- gnucash.orig/src/backend/file/io-gncxml-v2.c
+++ gnucash/src/backend/file/io-gncxml-v2.c
@@ -302,16 +302,6 @@ add_pricedb_local(sixtp_gdv2 *data, GNCP
     return TRUE;
 }
 
-#if 0
-static gboolean
-add_budget_local(sixtp_gdv2 *data, GncBudget *bgt)
-{
-/* CAS:I don't think anything is needed here, because budgets are
- * automatically added to their book's collections when they are created. */
-  return TRUE;
-}
-#endif
-
 static void
 do_counter_cb (const char *type, gpointer data_p, gpointer be_data_p)
 {
@@ -541,8 +531,7 @@ book_callback(const char *tag, gpointer 
     }
     else if(safe_strcmp(tag, BUDGET_TAG) == 0)
     {
-        // What's this for?  Needed?
-        //add_budget_local(gd, (GncBudget *)data);
+        // Nothing needed here.
     }
     else
     {
@@ -825,7 +814,7 @@ static void write_pricedb (FILE *out, Qo
 static void write_transactions (FILE *out, QofBook *book, sixtp_gdv2 *gd);
 static void write_template_transaction_data (FILE *out, QofBook *book, sixtp_gdv2 *gd);
 static void write_schedXactions(FILE *out, QofBook *book, sixtp_gdv2 *gd);
-static void write_budgets(FILE *out, QofBook *book, sixtp_gdv2 *gd);
+static void write_budget (QofEntity *ent, gpointer data);
 
 static void
 write_counts_cb (const char *type, gpointer data_p, gpointer be_data_p)
@@ -887,6 +876,7 @@ write_book(FILE *out, QofBook *book, six
 
     be_data.out = out;
     be_data.book = book;
+    be_data.gd = gd;
     if(fprintf( out, "<%s version=\"%s\">\n", BOOK_TAG, gnc_v2_book_version_string) < 0)
 	{
 		qof_backend_set_error(qof_book_get_backend(book), ERR_FILEIO_WRITE_ERROR);
@@ -907,11 +897,11 @@ write_book(FILE *out, QofBook *book, six
                  gnc_book_count_transactions(book),
                  "schedxaction",
                  g_list_length( gnc_book_get_schedxactions(book) ),
-		 "budget",
-		 g_list_length(gnc_book_get_budgets(book)),
+		 "budget", qof_collection_count(
+                     qof_book_get_collection(book, GNC_ID_BUDGET)),
 		 NULL);
 
-	qof_object_foreach_backend (GNC_FILE_BACKEND, write_counts_cb, &be_data);
+    qof_object_foreach_backend (GNC_FILE_BACKEND, write_counts_cb, &be_data);
 
     write_commodities(out, book, gd);
     write_pricedb(out, book, gd);
@@ -919,7 +909,10 @@ write_book(FILE *out, QofBook *book, six
     write_transactions(out, book, gd);
     write_template_transaction_data(out, book, gd);
     write_schedXactions(out, book, gd);
-    write_budgets(out, book, gd);
+
+    qof_collection_foreach(qof_book_get_collection(book, GNC_ID_BUDGET), 
+        write_budget, &be_data);
+
     qof_object_foreach_backend (GNC_FILE_BACKEND, write_data_cb, &be_data);
 
     if(fprintf( out, "</%s>\n", BOOK_TAG ) < 0) {
@@ -1070,26 +1063,19 @@ write_schedXactions( FILE *out, QofBook 
 }
 
 static void
-write_budgets( FILE *out, QofBook *book, sixtp_gdv2 *gd)
+write_budget (QofEntity *ent, gpointer data)
 {
-    GList *budgetList;
-    GncBudget *tmp;
     xmlNodePtr node;
+    struct file_backend* be = data;
 
-    /* get list of budgets from QofBook */
-    budgetList = gnc_book_get_budgets(book);
-
-    g_return_if_fail(budgetList);
-
-    do {
-        tmp = budgetList->data;
-        node = gnc_budget_dom_tree_create( tmp );
-        xmlElemDump( out, NULL, node );
-        fprintf( out, "\n" );
-        xmlFreeNode( node );
-        gd->counter.budgets_loaded++;
-        run_callback(gd, "budgets");
-    } while ((budgetList = budgetList->next));
+    GncBudget *bgt = GNC_BUDGET(ent);
+    node = gnc_budget_dom_tree_create(bgt);
+    xmlElemDump( be->out, NULL, node );
+    fprintf( be->out, "\n" );
+    xmlFreeNode( node );
+    
+    be->gd->counter.budgets_loaded++;
+    run_callback(be->gd, "budgets");    
 }
 
 void
@@ -1160,7 +1146,9 @@ gnc_book_write_to_xml_filehandle_v2(QofB
     gd->counter.transactions_total = gnc_book_count_transactions(book);
     gd->counter.schedXactions_total =
       g_list_length( gnc_book_get_schedxactions(book));
-    gd->counter.budgets_total = g_list_length(gnc_book_get_budgets(book));
+    gd->counter.budgets_total = qof_collection_count(
+        qof_book_get_collection(book, GNC_ID_BUDGET));
+
     write_book(out, book, gd);
 
     fprintf(out, "</" GNC_V2_STRING ">\n\n");
Index: gnucash/src/engine/qofid.c
===================================================================
--- gnucash.orig/src/engine/qofid.c
+++ gnucash/src/engine/qofid.c
@@ -388,20 +388,4 @@ qof_collection_foreach (QofCollection *c
   g_hash_table_foreach (col->hash_of_entities, foreach_cb, &iter);
 }
 
-
-static void
-add_to_list(gpointer key, gpointer val, gpointer data) {
-    GList **list = data;
-    *list = g_list_append(*list, QOF_ENTITY(val));
-}
-
-GList *
-qof_collection_get_list(QofCollection *col) {
-    GList *list = NULL;
-    g_return_val_if_fail (col, NULL);
-
-    g_hash_table_foreach(col->hash_of_entities, add_to_list, &list);
-    return list;
-}
-
 /* =============================================================== */
Index: gnucash/src/engine/qofid.h
===================================================================
--- gnucash.orig/src/engine/qofid.h
+++ gnucash/src/engine/qofid.h
@@ -182,9 +182,6 @@ typedef void (*QofEntityForeachCB) (QofE
 void qof_collection_foreach (QofCollection *, QofEntityForeachCB,
                              gpointer user_data);
 
-/** Get a list of entities in the collection.  Caller owns the list. */
-GList * qof_collection_get_list(QofCollection *col);
-
 /** Store and retrieve arbitrary object-defined data
  *
  * XXX We need to add a callback for when the collection is being


More information about the gnucash-patches mailing list