52a4d1626cac96ed654c5f09b9d6ab60ed9581b5
tdreszer
  Thu Aug 12 17:08:17 2010 -0700
Keeping track of vars chasnged by cgi requires the concept of CART_VAR_EMPTY
diff --git src/hg/lib/cart.c src/hg/lib/cart.c
index 3b0aa44..565ca39 100644
--- src/hg/lib/cart.c
+++ src/hg/lib/cart.c
@@ -287,6 +287,15 @@
 if (oldVars == NULL)
     return;
 struct hashEl *hel = hashLookup(cart->hash, var);
+
+#define CART_DIFFS_INCLUDE_EMPTIES
+#ifdef CART_DIFFS_INCLUDE_EMPTIES
+// NOTE: New cgi vars not in old cart cannot be distinguished from vars not newly set by cgi.
+//       Solution: Add 'empty' var to old vars for cgi vars not already in cart
+if (hel == NULL)
+    hashAdd(oldVars, var, cloneString(CART_VAR_EMPTY));
+#endif///def CART_DIFFS_INCLUDE_EMPTIES
+
 while (hel != NULL)
     {
     hashAdd(oldVars, var, cloneString(hel->val));
@@ -313,8 +322,7 @@
         char *suffix = el->name + suffixOffset;
         if(sameString(suffix,POSITION_SUFFIX))
             {
-            struct hashEl *old = hashLookup(oldVars, el->name);
-            if(old != NULL && differentString((char *)old->val,(char *)el->val)) // NOTE: It seems NULL is only seen when cart is cleared and this is not important
+            if (cartValueHasChanged(cart,oldVars,el->name,TRUE,TRUE))
                 {
                 char *name = cloneString(el->name);
                 safecpy(name+suffixOffset,strlen(POSITION_SUFFIX),IMGORD_SUFFIX); // We know that POSITION_SUFFIX is longer than IMGORD_SUFFIX
@@ -374,6 +382,7 @@
 	{
 	storeInOldVars(cart, oldVars, cv->name);
 	cartRemove(cart, cv->name);
+        if (differentString(cv->val, CART_VAR_EMPTY))  // NOTE: CART_VAR_EMPTY logic not implemented for boolShad or multiShad
 	hashAdd(cgiHash, cv->name, cv->val);
 	}
     }
@@ -1926,16 +1935,31 @@
 }
 
 
-boolean cartValueHasChanged(struct cart *newCart,struct hash *oldVars,char *setting,boolean ignoreRemoved)
+boolean cartValueHasChanged(struct cart *newCart,struct hash *oldVars,char *setting,boolean ignoreRemoved,boolean ignoreCreated)
 /* Returns TRUE if new cart setting has changed from old cart setting */
 {
-char *newValue = cartOptionalString(newCart,setting);
 char *oldValue = hashFindVal(oldVars,setting);
+if (oldValue == NULL)
+#ifdef CART_DIFFS_INCLUDE_EMPTIES
+    return FALSE;  // All vars changed by cgi will be found in old vars
+#else///ifndef CART_DIFFS_INCLUDE_EMPTIES
+    return (!ignoreCreated);
+#endif///ndef CART_DIFFS_INCLUDE_EMPTIES
 
+char *newValue = cartOptionalString(newCart,setting);
 if (newValue == NULL)
-    return (!ignoreRemoved && oldValue != NULL);
-if (oldValue == NULL)
-    return FALSE;  // FIXME: This seems strange but it is what works in practice.
+    return (!ignoreRemoved);
+
+#ifdef CART_DIFFS_INCLUDE_EMPTIES
+if (sameString(oldValue,CART_VAR_EMPTY))
+    {
+    if (sameString(newValue,"hide")
+    ||  sameString(newValue,"off")
+    ||  sameString(newValue,"0"))   // Special cases DANGER!
+        return FALSE;
+    }
+#endif///def CART_DIFFS_INCLUDE_EMPTIES
+
 return (differentString(newValue,oldValue));
 }
 
@@ -1980,7 +2004,7 @@
 struct slRef *oneName = NULL;
 while ((oneName = slPopHead(&oldList)) != NULL)
     {
-    boolean thisOneChanged = cartValueHasChanged(newCart,oldVars,oneName->val,ignoreRemoved);
+    boolean thisOneChanged = cartValueHasChanged(newCart,oldVars,oneName->val,ignoreRemoved,TRUE);
     if (unChanged != thisOneChanged)
         slAddHead(&newList,oneName);
     else
@@ -1999,15 +2023,16 @@
    If suffix NULL then removes 'trackName' which holds visibility */
 {
 int removed = 0;
+boolean vis = (suffix == NULL || *suffix == '\0');
 struct slRef *tdbRef, *tdbRefList = trackDbListGetRefsToDescendants(skipParent?tdb->subtracks:tdb);
 for (tdbRef = tdbRefList; tdbRef != NULL; tdbRef = tdbRef->next)
     {
     struct trackDb *descendentTdb = tdbRef->val;
     char settingName[512];  // wgEncodeOpenChromChip.Peaks.vis
-    if (suffix != NULL)
-        safef(settingName,sizeof(settingName),"%s.%s",descendentTdb->track,suffix);
-    else
+    if (vis)
         safef(settingName,sizeof(settingName),"%s",descendentTdb->track);
+    else
+        safef(settingName,sizeof(settingName),"%s.%s",descendentTdb->track,suffix);
     removed += cartRemoveAndCount(cart,settingName);
     }
 return removed;
@@ -2021,32 +2046,51 @@
 if (!tdbIsComposite(tdb))
     return FALSE;
 
+// vis is a special additive case! composite or view level changes then remove subtrack vis
+boolean compositeVisChanged = cartValueHasChanged(newCart,oldVars,tdb->track,TRUE,TRUE);
+//boolean compositeVisHidden  = FALSE;
+//if(compositeVisChanged)
+//    compositeVisHidden = (cartUsualInt(cart,tdb->track,tvFull) == tvHide);
+
+
+// FIXME: Big problem in persistence of changed state.
+boolean debug = FALSE;//sameString(tdb->track,"wgEncodeBroadChipSeq");
+if(debug)
+    {
+    char *newValue = cartOptionalString(newCart,tdb->track);
+    char *oldValue = hashFindVal(oldVars,tdb->track);
+    warn("Cleanup: wgEncodeBroadChipSeq  compositeVisChanged:%s  new:%s  old:%s",
+         (compositeVisChanged?"yes":"no"),(newValue!=NULL?newValue:"(null)"),(oldValue!=NULL?oldValue:"(null)"));
+    }
+
 // Build list of current settings for composite
 char setting[512];
 safef(setting,sizeof(setting),"%s.",tdb->track);
 struct slRef *changedSettings = cartNamesPrefixedBy(newCart, setting);
-if (changedSettings == NULL)
+if (changedSettings == NULL && !compositeVisChanged)
     return FALSE;
 
 // Prune list to only those which have changed
+if(changedSettings != NULL)
+    {
+    if(debug) warn("Cleanup: settings:%d",slCount(changedSettings));
 (void)cartNamesPruneChanged(newCart,oldVars,&changedSettings,TRUE,FALSE);
-if (changedSettings == NULL)
+    if (changedSettings == NULL && !compositeVisChanged)
     return FALSE;
+    if(debug) warn("Cleanup: changed:%d",changedSettings==NULL?0:slCount(changedSettings));
+    }
 
 struct slRef *oneName = NULL;
 char * var = NULL;
 boolean clensed = FALSE;
 
-// vis is a special additive case! composite or view level changes then remove subtrack vis
-boolean compositeVisChanged = cartValueHasChanged(newCart,oldVars,tdb->track,TRUE);
-
-
 // Walk through views
 boolean hasViews = FALSE;
 struct trackDb *tdbView = tdb->subtracks;
 for (;tdbView != NULL; tdbView = tdbView->next)
     {
     boolean viewVisChanged = FALSE;
+//    boolean viewVisHidden  = FALSE;
     char * view = NULL;
     if (!tdbIsView(tdbView,&view))
         break;
@@ -2063,7 +2107,10 @@
             {
             var = oneName->val + strlen(setting);
             if (sameString(var,"vis"))
+                {
                 viewVisChanged = TRUE;
+//                viewVisHidden = (cartUsualInt(cart,oneName->val,tvFull) == tvHide);
+                }
             else {
                 if (cartRemoveFromTdbTree(newCart,tdbView,var,TRUE) > 0)
                     clensed = TRUE;