> Looks pretty good. I might add an undo boundary
> around the whole thing (I note emacs doesn't do this
> properly, at least on the version I have here)...
I like it.  If undo is a concern, then it might be a good idea to
check the other functions while we're here.

I found at least the following offenders in random.c:
twiddle() might return early, leaving the boundaries disabled;
openline() can open many at once, but each gets its own boundary;
justone() makes two changes but doesn't set the boundary;
lfindent() and newline() have the same problem as openline().

There's also indent() which makes two changes, but it's not bound
to a key and is only called via cmode, where the undo boundaries
are in place already.

I'm afraid simply adding the the undo boundary around newline()
will break yank(), which sets its own boundary and calls newline()
among other changes.  If we want this undo stuff, then we probably
should add checks such that none of these functions set boundaries
if they were disabled (by some other function) to start with.  What
do you think?

The following diff fixes twiddle() and adds boundaries for openline(),
justone(), and lfindent().  I leave newline() intact for now.

--- src/usr.bin/mg.old/random.c Mon Jan 17 15:16:09 2011
+++ src/usr.bin/mg/random.c     Mon Jan 17 16:25:21 2011
@@ -119,7 +119,6 @@ twiddle(int f, int n)
 
        dotp = curwp->w_dotp;
        doto = curwp->w_doto;
-       undo_boundary_enable(FFRAND, 0);
        if (doto == llength(dotp)) {
                if (--doto <= 0)
                        return (FALSE);
@@ -129,6 +128,7 @@ twiddle(int f, int n)
                if (doto == 0)
                        return (FALSE);
        }
+       undo_boundary_enable(FFRAND, 0);
        cr = lgetc(dotp, doto - 1);
        (void)backdel(FFRAND, 1);
        (void)forwchar(FFRAND, 1);
@@ -158,6 +158,7 @@ openline(int f, int n)
                return (TRUE);
 
        /* insert newlines */
+       undo_boundary_enable(FFRAND, 0);
        i = n;
        do {
                s = lnewline();
@@ -166,6 +167,7 @@ openline(int f, int n)
        /* then go back up overtop of them all */
        if (s == TRUE)
                s = backchar(f | FFRAND, n);
+       undo_boundary_enable(FFRAND, 1);
        return (s);
 }
 
@@ -223,8 +225,11 @@ deblank(int f, int n)
 int
 justone(int f, int n)
 {
+       undo_boundary_enable(FFRAND, 0);
        (void)delwhite(f, n);
-       return (linsert(1, ' '));
+       linsert(1, ' ');
+       undo_boundary_enable(FFRAND, 1);
+       return (TRUE);
 }
 
 /*
@@ -318,10 +323,12 @@ int
 lfindent(int f, int n)
 {
        int     c, i, nicol;
+       int     s = TRUE;
 
        if (n < 0)
                return (FALSE);
 
+       undo_boundary_enable(FFRAND, 0);
        while (n--) {
                nicol = 0;
                for (i = 0; i < llength(curwp->w_dotp); ++i) {
@@ -337,10 +344,13 @@ lfindent(int f, int n)
                    curbp->b_flag & BFNOTAB) ? linsert(nicol, ' ') == FALSE : (
 #endif /* NOTAB */
                    ((i = nicol / 8) != 0 && linsert(i, '\t') == FALSE) ||
-                   ((i = nicol % 8) != 0 && linsert(i, ' ') == FALSE))))
-                       return (FALSE);
+                   ((i = nicol % 8) != 0 && linsert(i, ' ') == FALSE)))) {
+                       s = FALSE;
+                       break;
+               }
        }
-       return (TRUE);
+       undo_boundary_enable(FFRAND, 1);
+       return (s);
 }
 
 /*

Reply via email to