Yeah, nice catch on the twiddle bug. This looks pretty good. Anyone else try it?
-kj On Mon, Jan 17, 2011 at 10:10 AM, Henri Kemppainen <[email protected]> wrote: > > 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); > } > > /*
