Complete new style file.Buffer implementation

This CL completes Edwood's ability to use the file.Buffer
implementation and passes all tests with the new Buffer
implementation. Helps with #97. Fixes some subtle bugs detected by
unit tests.

exec_test.go

@@ -323,16 +323,24 @@ func mutateBothOtherUndo(t *testing.T, g *globals) {
 	firstwin := g.row.col[0].w[0]
 	secondwin := g.row.col[0].w[1]
 
+	t.Logf("firstwin, %q", firstwin.body.file.String())
+	t.Logf("secondwin, %q", secondwin.body.file.String())
+
 	// Modify the firstwin.
 	firstwin.body.q0 = 3
 	firstwin.body.q1 = 10
 	global.seq++
 	firstwin.body.file.Mark(global.seq)
 	cut(&firstwin.tag, &firstwin.body, nil, false, true, "")
 
+	t.Logf("after cut firstwin, %q", firstwin.body.file.String())
+
 	// Run undo from one of the windows. (i.e. same as clicking on the Undo action.)
 	// Cut should remain, original global edit should get Undone only in secondwin.
 	undo(&secondwin.tag, nil, nil, true /* this is an undo */, false /* ignored */, "")
+
+	t.Logf("after undo firstwin, %q", firstwin.body.file.String())
+	t.Logf("after undo secondwin, %q", secondwin.body.file.String())
 }
 
 func mutateBranchedAndRejoined(t *testing.T, g *globals) {

file/buffer_adapter.go

@@ -3,7 +3,6 @@ package file
 import (
 	"bufio"
 	"bytes"
-	"flag"
 	"io"
 	"strings"
 )
@@ -73,17 +72,9 @@ type BufferAdapter interface {
 // Enforce that *file.File implements BufferAdapter.
 var (
 	_ BufferAdapter = (*File)(nil)
-
-	// TODO(rjk): Make this compile. :-)
 	_ BufferAdapter = (*Buffer)(nil)
-
-	newTypeBuffer bool
 )
 
-func init() {
-	flag.BoolVar(&newTypeBuffer, "newtypebuffer", false, "turn on the file.Buffer new Buffer implementation")
-}
-
 func NewTypeBuffer(inputrunes []rune, oeb *ObservableEditableBuffer) BufferAdapter {
 	// TODO(rjk): Figure out how to plumb in the oeb object to setup Undo
 	// observer callbacks.

file/file_file_test.go

@@ -36,7 +36,7 @@ func TestFileHandlesNilEpsilonDelta(t *testing.T) {
 		{"undo (nil delta)", true, 14, 17, 14, 17, nil, nil},
 		{"redo (nil epsilon)", false, 14, 17, 14, 17, nil, nil},
 	} {
-		oeb := MakeObservableEditableBuffer("", []rune("This is an example sentence.\n"))
+		oeb := _makeObservableEditableBuffer("", []rune("This is an example sentence.\n"), false)
 
 		fimpl := oeb.f.(*File)
 		fimpl.delta = tc.delta

file/file_test.go

@@ -137,15 +137,33 @@ func TestFileUndoRedo(t *testing.T) {
 			// Because of how seq managed the number of Undo actions, this
 			// corresponds to the case of not incrementing seq and undoes every
 			// action in the log.
-			f.Undo(true)
+			f.checkedUndo(true, t, undoexpectation{
+				ok: true,
+				q0: 0,
+				q1: 0,
+			})
 
 			t.Log(f.f.String())
 			t.Log(f.seq, f.f.HasUncommitedChanges(), f.Dirty(), f.putseq)
 			check(t, "TestFileUndoRedo after 1 undo", f,
 				&stateSummary{false, false, true, false, ""})
 
 			// Redo
-			f.Undo(false)
+			// This is actually wrong with legacy file. It will leave a portion of the selection
+			// set.
+			if nt == "newtype" {
+				f.checkedUndo(false, t, undoexpectation{
+					ok: true,
+					q0: 0,
+					q1: 9,
+				})
+			} else {
+				f.checkedUndo(false, t, undoexpectation{
+					ok: true,
+					q0: 6,
+					q1: 9,
+				})
+			}
 
 			t.Log(f.f.String())
 			t.Log(f.seq, f.f.HasUncommitedChanges(), f.Dirty(), f.putseq)
@@ -175,13 +193,21 @@ func TestFileUndoRedoWithMark(t *testing.T) {
 			check(t, "TestFileUndoRedoWithMark after 2 inserts", f,
 				&stateSummary{false, true, false, true, s1 + s2})
 
-			f.Undo(true)
+			f.checkedUndo(true, t, undoexpectation{
+				ok: true,
+				q0: 6,
+				q1: 6,
+			})
 
 			check(t, "TestFileUndoRedoWithMark after 1 undo", f,
 				&stateSummary{false, true, true, true, s1})
 
 			// Redo
-			f.Undo(false)
+			f.checkedUndo(false, t, undoexpectation{
+				ok: true,
+				q0: 6,
+				q1: 9,
+			})
 
 			check(t, "TestFileUndoRedoWithMark after 1 redo", f,
 				&stateSummary{false, true, false, true, s1 + s2})
@@ -274,7 +300,9 @@ func TestFileLoadUndoHash(t *testing.T) {
 			}
 
 			// Undo renmaing the file.
-			f.Undo(true)
+			f.checkedUndo(true, t, undoexpectation{
+				ok: false,
+			})
 			check(t, "TestFileLoadUndoHash after Undo", f,
 				&stateSummary{false, false, true, false, s2 + s2})
 			if got, want := f.Name(), "edwood"; got != want {
@@ -328,7 +356,11 @@ func TestFileInsertDeleteUndo(t *testing.T) {
 				&stateSummary{false, true, false, true, "yi 海老hi 海老麺麺"})
 			t.Logf("after setup seq %d, putseq %d", f.seq, f.putseq)
 
-			f.Undo(true)
+			f.checkedUndo(true, t, undoexpectation{
+				ok: true,
+				q0: 5,
+				q1: 5,
+			})
 			check(t, "TestFileInsertDeleteUndo after 1 Undo", f,
 				&stateSummary{false, true, true, true, "yi 海老麺"})
 			t.Logf("after 1 Undo seq %d, putseq %d", f.seq, f.putseq)
@@ -365,12 +397,21 @@ func TestFileInsertDeleteUndo(t *testing.T) {
 				},
 			})
 
-			f.Undo(true) // 2 deletes should get removed because they have the same sequence.
+			// 2 deletes should get removed because they have the same sequence.
+			f.checkedUndo(true, t, undoexpectation{
+				ok: true,
+				q0: 0,
+				q1: 1,
+			})
 			check(t, "TestFileInsertDeleteUndo after 2 Undo", f,
 				&stateSummary{false, true, true, true, "byehi 海老麺"})
 			t.Logf("after 2 Undo seq %d, putseq %d", f.seq, f.putseq)
 
-			f.Undo(false) // 2 deletes should be put back.
+			f.checkedUndo(false, t, undoexpectation{ // 2 deletes should be put back.
+				ok: true,
+				q0: 1,
+				q1: 1,
+			})
 			check(t, "TestFileInsertDeleteUndo after 1 Undo", f,
 				&stateSummary{false, true, true, true, "yi 海老麺"})
 			t.Logf("after 1 Redo seq %d, putseq %d", f.seq, f.putseq)
@@ -421,7 +462,11 @@ func TestFileRedoSeq(t *testing.T) {
 				t.Errorf("TestFileRedoSeq no redo. got %#v want %#v", got, want)
 			}
 
-			f.Undo(true)
+			f.checkedUndo(true, t, undoexpectation{
+				ok: true,
+				q0: 0,
+				q1: 0,
+			})
 			check(t, "TestFileRedoSeq after Undo", f,
 				&stateSummary{false, false, true, false, ""})
 
@@ -535,7 +580,9 @@ func TestFileNameSettingWithScratch(t *testing.T) {
 				t.Errorf("TestFileNameSettingWithScratch failed to init isscratch. got %v want %v", got, want)
 			}
 
-			f.Undo(true)
+			f.checkedUndo(true, t, undoexpectation{
+				ok: false,
+			})
 
 			if got, want := f.Name(), "/guide"; got != want {
 				t.Errorf("TestFileNameSettingWithScratch failed to init name. got %v want %v", got, want)
@@ -544,7 +591,9 @@ func TestFileNameSettingWithScratch(t *testing.T) {
 				t.Errorf("TestFileNameSettingWithScratch failed to init isscratch. got %v want %v", got, want)
 			}
 
-			f.Undo(true)
+			f.checkedUndo(true, t, undoexpectation{
+				ok: false,
+			})
 			if got, want := f.Name(), "edwood"; got != want {
 				t.Errorf("TestFileNameSettingWithScratch failed to init name. got %v want %v", got, want)
 			}
@@ -617,12 +666,20 @@ func TestTagObserversFireCorrectly(t *testing.T) {
 				t.Errorf("got %+v, want %+v", got, want)
 			}
 
-			oeb.Undo(true)
+			oeb.checkedUndo(true, t, undoexpectation{
+				ok: true,
+				q0: 0,
+				q1: 0,
+			})
 			if got, want := *counts, (observercount{0, 5}); got != want {
 				t.Errorf("got %+v, want %+v", got, want)
 			}
 
-			oeb.Undo(false)
+			oeb.checkedUndo(false, t, undoexpectation{
+				ok: true,
+				q0: 0,
+				q1: 2,
+			})
 			if got, want := *counts, (observercount{0, 6}); got != want {
 				t.Errorf("got %+v, want %+v", got, want)
 			}

file/helpers_test.go

@@ -145,3 +145,31 @@ func (s *span) String() string {
 	return buffy.String()
 
 }
+
+type undoexpectation struct {
+	q0 int
+	q1 int
+	ok bool
+}
+
+func (e *ObservableEditableBuffer) checkedUndo(isundo bool, t *testing.T, u undoexpectation) {
+	t.Helper()
+
+	q0, q1, ok := e.Undo(isundo)
+
+	if got, want := ok, u.ok; got != want {
+		t.Errorf("Undo wrong ok: got %v, want %v", got, want)
+	}
+
+	if !ok {
+		// Values of q0, q1 don't matter if ok is false
+		return
+	}
+
+	if got, want := q0, u.q0; got != want {
+		t.Errorf("Undo wrong q0: got %d, want %d", got, want)
+	}
+	if got, want := q1, u.q1; got != want {
+		t.Errorf("Undo wrong q1: got %d, want %d", got, want)
+	}
+}

file/observable_editable_buffer.go

@@ -131,7 +131,7 @@ func (e *ObservableEditableBuffer) HasMultipleObservers() bool {
 
 // MakeObservableEditableBuffer is a constructor wrapper for NewFile() to abstract File from the main program.
 func MakeObservableEditableBuffer(filename string, b []rune) *ObservableEditableBuffer {
-	return _makeObservableEditableBuffer(filename, b, newTypeBuffer)
+	return _makeObservableEditableBuffer(filename, b, true)
 }
 
 func _makeObservableEditableBuffer(filename string, b []rune, newtype bool) *ObservableEditableBuffer {