Minimize copy_to_undostruct calls

5 files changed, 98 insertions, 120 deletions

diff --git a/src/cmds_normal.c b/src/cmds_normal.c
index 6fb0b4e..4251ccf 100644
--- a/src/cmds_normal.c
+++ b/src/cmds_normal.c
@@ -621,20 +621,19 @@ void do_normalmode(struct block * buf) {
 
                     // added for #244 - 22/03/2018
                     ents_that_depends_on_range(n->row, n->col, n->row, n->col);
-                    if (deps != NULL) {
-                        for (i = 0; i < deps->vf; i++) {
+
 #ifdef UNDO
+                    if (deps != NULL)
+                        for (i = 0; i < deps->vf; i++)
                             copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_DEL);
 #endif
-                            EvalJustOneVertex(deps[i].vp, deps[i].vp->row, deps[i].vp->col, 0);
+
+                    EvalJustOneVertex(deps[i].vp, deps[i].vp->row, deps[i].vp->col, 0);
 #ifdef UNDO
+                    if (deps != NULL)
+                        for (i = 0; i < deps->vf; i++)
                             copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_ADD);
 #endif
-                            }
-                    }
-
-
-
 
                 }
 #ifdef UNDO
diff --git a/src/gram.y b/src/gram.y
index 0791b20..7f4f390 100755
--- a/src/gram.y
+++ b/src/gram.y
@@ -424,10 +424,9 @@ command:
                                   extern struct ent_ptr * deps;
                                   int i, n = 0;
                                   ents_that_depends_on_range($2.left.vp->row, $2.left.vp->col, $2.left.vp->row, $2.left.vp->col);
-                                  if (deps != NULL) {
+                                  if (deps != NULL)
                                       for (i = 0, n = deps->vf; i < n; i++)
                                           copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_DEL);
-                                  }
                                   #endif
                                   if (getVertex(graph, lookat($2.left.vp->row, $2.left.vp->col), 0) != NULL) destroy_vertex(lookat($2.left.vp->row, $2.left.vp->col));
 
@@ -443,13 +442,11 @@ command:
 
                                   // clearing the value counts as a write, so run write triggers
                                   if (( $2.left.vp->trigger  ) && (($2.left.vp->trigger->flag & TRG_WRITE) == TRG_WRITE))
-                                      do_trigger($2.left.vp,TRG_WRITE);
+                                      do_trigger($2.left.vp, TRG_WRITE);
 
                                   #ifdef UNDO
                                   copy_to_undostruct($2.left.vp->row, $2.left.vp->col, $2.left.vp->row, $2.left.vp->col, UNDO_ADD);
                                   // here we save in undostruct, all the ents that depends on the deleted one (after change)
-                                  if (deps != NULL) free(deps);
-                                  ents_that_depends_on_range($2.left.vp->row, $2.left.vp->col, $2.left.vp->row, $2.left.vp->col);
                                   if (deps != NULL) {
                                       for (i = 0, n = deps->vf; i < n; i++)
                                           copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_ADD);
diff --git a/src/interp.c b/src/interp.c
index dd2ca55..b26ca62 100644
--- a/src/interp.c
+++ b/src/interp.c
@@ -2340,16 +2340,14 @@ void fill(struct ent *v1, struct ent *v2, double start, double inc) {
     if (minr < 0) minr = 0;
     if (minc < 0) minc = 0;
 
-    #ifdef UNDO
+#ifdef UNDO
     create_undo_action();
-    #endif
+    copy_to_undostruct(minr, minc, maxr, maxc, UNDO_DEL);
+#endif
 
     if (calc_order == BYROWS) {
         for (r = minr; r<=maxr; r++)
             for (c = minc; c<=maxc; c++) {
-                #ifdef UNDO
-                copy_to_undostruct(r, c, r, c, UNDO_DEL);
-                #endif
                 n = lookat(r, c);
                 if (n->flags&is_locked) continue;
                 (void) clearent(n);
@@ -2357,34 +2355,26 @@ void fill(struct ent *v1, struct ent *v2, double start, double inc) {
                 start += inc;
                 n->flags |= (is_changed|is_valid);
                 n->flags &= ~(iscleared);
-                #ifdef UNDO
-                copy_to_undostruct(r, c, r, c, UNDO_ADD);
-                #endif
             }
     }
     else if (calc_order == BYCOLS) {
         for (c = minc; c<=maxc; c++)
             for (r = minr; r<=maxr; r++) {
-                #ifdef UNDO
-                copy_to_undostruct(r, c, r, c, UNDO_DEL);
-                #endif
                 n = lookat(r, c);
                 (void) clearent(n);
                 n->v = start;
                 start += inc;
                 n->flags |= (is_changed|is_valid);
                 n->flags &= ~(iscleared);
-                #ifdef UNDO
-                copy_to_undostruct(r, c, r, c, UNDO_ADD);
-                #endif
             }
     }
     else {
         sc_error(" Internal error calc_order");
     }
-    #ifdef UNDO
+#ifdef UNDO
+    copy_to_undostruct(minr, minc, maxr, maxc, UNDO_ADD);
     end_undo_action();
-    #endif
+#endif
 }
 
 /**
@@ -2412,23 +2402,19 @@ void lock_cells(struct ent * v1, struct ent * v2) {
     if (minr < 0) minr = 0;
     if (minc < 0) minc = 0;
 
-    #ifdef UNDO
+#ifdef UNDO
     create_undo_action();
-    #endif
+    copy_to_undostruct(minr, minc, maxr, maxc, UNDO_DEL);
+#endif
     for (r = minr; r <= maxr; r++)
         for (c = minc; c <= maxc; c++) {
             n = lookat(r, c);
-    #ifdef UNDO
-            copy_to_undostruct(r, c, r, c, UNDO_DEL);
-    #endif
             n->flags |= is_locked;
-    #ifdef UNDO
-            copy_to_undostruct(r, c, r, c, UNDO_ADD);
-    #endif
         }
-    #ifdef UNDO
+#ifdef UNDO
+    copy_to_undostruct(minr, minc, maxr, maxc, UNDO_ADD);
     end_undo_action();
-    #endif
+#endif
 }
 
 /**
@@ -2456,23 +2442,19 @@ void unlock_cells(struct ent * v1, struct ent * v2) {
     if (minr < 0) minr = 0;
     if (minc < 0) minc = 0;
 
-    #ifdef UNDO
+#ifdef UNDO
     create_undo_action();
-    #endif
+    copy_to_undostruct(minr, minc, maxr, maxc, UNDO_DEL);
+#endif
     for (r = minr; r <= maxr; r++)
         for (c = minc; c <= maxc; c++) {
             n = lookat(r, c);
-    #ifdef UNDO
-            copy_to_undostruct(r, c, r, c, UNDO_DEL);
-    #endif
             n->flags &= ~is_locked;
-    #ifdef UNDO
-            copy_to_undostruct(r, c, r, c, UNDO_ADD);
-    #endif
         }
-    #ifdef UNDO
+#ifdef UNDO
+    copy_to_undostruct(minr, minc, maxr, maxc, UNDO_ADD);
     end_undo_action();
-    #endif
+#endif
 }
 
 /* set the numeric part of a cell */
@@ -2498,10 +2480,9 @@ void let(struct ent * v, struct enode * e) {
 
         // here we save in undostruct, all the ents that depends on the deleted one (before change)
         ents_that_depends_on_range(v->row, v->col, v->row, v->col);
-        if (deps != NULL) {
+        if (deps != NULL)
             for (i = 0; i < deps->vf; i++)
                 copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_DEL);
-        }
     }
     #endif
     //if (getVertex(graph, lookat(v->row, v->col), 0) != NULL) {
@@ -2512,7 +2493,6 @@ void let(struct ent * v, struct enode * e) {
         // y en esta llamada a let estamos haciendo let F5 = F1 + F3
     //}
 
-
     double val;
     unsigned isconstant = constant(e);
     if (v->row == currow && v->col == curcol)
@@ -2579,8 +2559,6 @@ void let(struct ent * v, struct enode * e) {
     if (!loading) {
         copy_to_undostruct(v->row, v->col, v->row, v->col, UNDO_ADD);
         // here we save in undostruct, all the ents that depends on the deleted one (after change)
-        if (deps != NULL) free(deps);
-        ents_that_depends_on_range(v->row, v->col, v->row, v->col);
         if (deps != NULL) {
             for (i = 0; i < deps->vf; i++)
                 copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_ADD);
@@ -2611,19 +2589,18 @@ void slet(struct ent * v, struct enode * se, int flushdir) {
     extern struct ent_ptr * deps;
     int i;
     if (!loading) {
-    create_undo_action();
-    copy_to_undostruct(v->row, v->col, v->row, v->col, UNDO_DEL);
+        create_undo_action();
+        copy_to_undostruct(v->row, v->col, v->row, v->col, UNDO_DEL);
 
-    // here we save in undostruct, all the ents that depends on the deleted one (before change)
-    ents_that_depends_on_range(v->row, v->col, v->row, v->col);
-    for (i = 0; deps != NULL && i < deps->vf; i++)
-        copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_DEL);
+        // here we save in undostruct, all the ents that depends on the deleted one (before change)
+        ents_that_depends_on_range(v->row, v->col, v->row, v->col);
+        for (i = 0; deps != NULL && i < deps->vf; i++)
+            copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_DEL);
     }
     #endif
     // No debe borrarse el vertex. Ver comentario en LET
     //if (getVertex(graph, lookat(v->row, v->col), 0) != NULL) destroy_vertex(lookat(v->row, v->col));
 
-
     char * p;
     if (v->row == currow && v->col == curcol) cellassign = 1;
     exprerr = 0;
@@ -2677,17 +2654,15 @@ void slet(struct ent * v, struct enode * se, int flushdir) {
 
     #ifdef UNDO
     if (!loading) {
-    copy_to_undostruct(v->row, v->col, v->row, v->col, UNDO_ADD);
-    // here we save in undostruct, all the ents that depends on the deleted one (after change)
-    if (deps != NULL) free(deps);
-    ents_that_depends_on_range(v->row, v->col, v->row, v->col);
-    if (deps != NULL) {
-        for (i = 0; i < deps->vf; i++)
-            copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_ADD);
-        free(deps);
-        deps = NULL;
-    }
-    end_undo_action();
+        copy_to_undostruct(v->row, v->col, v->row, v->col, UNDO_ADD);
+        // here we save in undostruct, all the ents that depends on the deleted one (after change)
+        if (deps != NULL) {
+            for (i = 0; i < deps->vf; i++)
+                copy_to_undostruct(deps[i].vp->row, deps[i].vp->col, deps[i].vp->row, deps[i].vp->col, UNDO_ADD);
+            free(deps);
+            deps = NULL;
+        }
+        end_undo_action();
     }
     #endif
 
@@ -3382,13 +3357,11 @@ int dateformat(struct ent *v1, struct ent *v2, char * fmt) {
 
     #ifdef UNDO
     create_undo_action();
+    copy_to_undostruct(minr, minc, maxr, maxc, UNDO_DEL);
     #endif
     for (r = minr; r <= maxr; r++) {
         for (c = minc; c <= maxc; c++) {
             n = lookat(r, c);
-    #ifdef UNDO
-            copy_to_undostruct(r, c, r, c, UNDO_DEL);
-    #endif
             if ( locked_cell(n->row, n->col) || ! (n)->label ) continue;
 
             // free all ent content but its label
@@ -3409,16 +3382,13 @@ int dateformat(struct ent *v1, struct ent *v2, char * fmt) {
             sprintf(s, "%c", 'd');
             strcat(s, fmt);
             n->format = s;
-
-    #ifdef UNDO
-            copy_to_undostruct(r, c, r, c, UNDO_ADD);
-    #endif
         }
     }
-    modflg++; // increase just one time
     #ifdef UNDO
-    end_undo_action();
+        copy_to_undostruct(minr, minc, maxr, maxc, UNDO_DEL);
+        end_undo_action();
     #endif
+    modflg++; // increase just one time
     return 0;
 }
 
diff --git a/src/undo.c b/src/undo.c
index 0ed0849..16294aa 100644
--- a/src/undo.c
+++ b/src/undo.c
@@ -391,7 +391,7 @@ void copy_to_undostruct (int row_desde, int col_desde, int row_hasta, int col_ha
             p = *ATBL(tbl, r, c);
             if (p == NULL) continue;
 
-            // here check that ent to add is not already in the list
+            /* here check that ent to add is not already in the list
             // if so, avoid to add a duplicate ent
             struct ent * lista = type == 'a' ? undo_item.added : undo_item.removed;
             repeated = 0;
@@ -403,8 +403,13 @@ void copy_to_undostruct (int row_desde, int col_desde, int row_hasta, int col_ha
                 lista = lista->next;
             }
             if (repeated) continue;
+            // is the above really neccesary? i believe its harmless to duplicate it.
+            // and no resources consuming..
+            */
 
             // not repeated - we malloc an ent and add it to list
+            // FIXME: improve this. ask memory for the whole range at once.
+            // do not malloc for every single cell in the range..
             struct ent * e = (struct ent *) malloc( (unsigned) sizeof(struct ent) );
             cleanent(e);
             copyent(e, lookat(r, c), 0, 0, 0, 0, 0, 0, 'u');
diff --git a/src/yank.c b/src/yank.c
index c71411f..4e4c947 100644
--- a/src/yank.c
+++ b/src/yank.c
@@ -101,6 +101,8 @@ struct ent * get_yanklist() {
 void free_yanklist () {
     if (yanklist == NULL) return;
     int c;
+
+    // free each ent internally
     struct ent * r = yanklist;
     struct ent * e;
     while (r != NULL) {
@@ -111,7 +113,7 @@ void free_yanklist () {
         if (r->expr) efree(r->expr);
         if (r->ucolor) free(r->ucolor);
 
-        free(r);
+        //free(r);
         r = e;
     }
 
@@ -121,50 +123,33 @@ void free_yanklist () {
         colformat[c] = NULL;
     }
 
+    // free yanklist
+    free(yanklist);
     yanklist = NULL;
     yanklist_tail = NULL;
     return;
 }
 
 /**
- * \brief Add 'ent' element to the yanklist
+ * \brief Add an already alloc'ed 'ent' element to the yanklist
  *
  * \param[in] item 'ent' element to add to the yanklist
  * \return none
  */
 
 void add_ent_to_yanklist(struct ent * item) {
-    // Create and initialize the 'ent'
-    struct ent * i_ent = (struct ent *) malloc (sizeof(struct ent));
-    (i_ent)->label = (char *)0;
-    (i_ent)->row = 0;
-    (i_ent)->col = 0;
-    (i_ent)->flags = may_sync;
-    (i_ent)->expr = (struct enode *)0;
-    (i_ent)->v = (double) 0.0;
-    (i_ent)->format = (char *)0;
-    (i_ent)->cellerror = CELLOK;
-    (i_ent)->next = NULL;
-    (i_ent)->ucolor = NULL;
-    (i_ent)->pad = 0;
-
-    // Copy 'item' content to 'i_ent'
-    (void) copyent(i_ent, item, 0, 0, 0, 0, 0, 0, 0);
-
-    (i_ent)->row = item->row;
-    (i_ent)->col = item->col;
 
    // If yanklist is empty, insert at the beginning
     if (yanklist == NULL) {
-        yanklist = i_ent;
-        yanklist_tail = i_ent;
+        yanklist = item;
+        yanklist_tail = item;
         return;
     }
 
     // If yanklist is NOT empty, insert at the end
     // insert at the end
-    yanklist_tail->next = i_ent;
-    yanklist_tail = i_ent;
+    yanklist_tail->next = item;
+    yanklist_tail = item;
     return;
 }
 
@@ -185,20 +170,40 @@ void add_ent_to_yanklist(struct ent * item) {
 
 void yank_area(int tlrow, int tlcol, int brrow, int brcol, char type, int arg) {
     int r,c;
-    free_yanklist();
     type_of_yank = type;
     yank_arg = arg;
+    free_yanklist();
+
+    struct ent * e_ori;
+    // ask for memory to keep struct ent * for the whole range
+    struct ent * y_cells = (struct ent *) calloc((brrow-tlrow+1)*(brcol-tlcol+1), sizeof(struct ent));
 
     for (r = tlrow; r <= brrow; r++)
         for (c = tlcol; c <= brcol; c++) {
-            struct ent * elm = *ATBL(tbl, r, c);
-
-            // Important: each 'ent' element keeps the corresponding row and col
-
-            //if (elm == NULL) elm = lookat(r, c);
-            //why create an empty ent where is not one?
-            //better this:
-            if (elm != NULL) add_ent_to_yanklist(elm);
+            e_ori = *ATBL(tbl, r, c);
+            if (e_ori != NULL) {
+                // Create and initialize the 'ent'
+                (y_cells)->label = (char *)0;
+                (y_cells)->row = 0;
+                (y_cells)->col = 0;
+                (y_cells)->flags = may_sync;
+                (y_cells)->expr = (struct enode *)0;
+                (y_cells)->v = (double) 0.0;
+                (y_cells)->format = (char *)0;
+                (y_cells)->cellerror = CELLOK;
+                (y_cells)->next = NULL;
+                (y_cells)->ucolor = NULL;
+                (y_cells)->pad = 0;
+
+                // Copy 'e_ori' content to 'y_cells'
+                (void) copyent(y_cells, e_ori, 0, 0, 0, 0, 0, 0, 0);
+
+                // Important: each 'ent' element keeps the corresponding row and col
+                (y_cells)->row = e_ori->row;
+                (y_cells)->col = e_ori->col;
+
+                add_ent_to_yanklist(y_cells++);
+            }
         }
     return;
 }
@@ -284,9 +289,12 @@ int paste_yanked_ents(int above, int type_paste) {
             int r = yll->row + diffr;
             int c = yll->col + diffc;
             checkbounds(&r, &c);
-            if (any_locked_cells(yll->row + diffr, yll->col + diffc, yll->row + diffr, yll->col + diffc))
-                // TODO: dismiss UNDO here
+            if (any_locked_cells(yll->row + diffr, yll->col + diffc, yll->row + diffr, yll->col + diffc)) {
+#ifdef UNDO
+                dismiss_undo_item(NULL);
+#endif
                 return -1;
+            }
             yll = yll->next;
         }
     }
@@ -298,7 +306,6 @@ int paste_yanked_ents(int above, int type_paste) {
 #ifdef UNDO
         copy_to_undostruct(yl->row + diffr, yl->col + diffc, yl->row + diffr, yl->col + diffc, UNDO_DEL);
 
-
         // save graph dependencies as well
         // added for #244 - 22/03/2018
         ents_that_depends_on_range(yl->row + diffr, yl->col + diffc, yl->row + diffr, yl->col + diffc);
@@ -308,7 +315,7 @@ int paste_yanked_ents(int above, int type_paste) {
         }
 #endif
 
-        // here we delete current content of "destino" ent
+        // here we delete current content of "destino" ent.
         if (type_paste == YANK_RANGE || type_paste == YANK_SORT)
             erase_area(yl->row + diffr, yl->col + diffc, yl->row + diffr, yl->col + diffc, ignorelock, 0);