Skip to content

Commit

Permalink
Fix bug in observer-driven tag updates
Browse files Browse the repository at this point in the history
My change to introduce observer-driven tag updates failed to correctly
propagate tag updates after doing a Put operation. Corrected this.
Helps eventually with #400.
  • Loading branch information
rjkroege committed Dec 2, 2021
1 parent ba10b30 commit e48e054
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 4 deletions.
3 changes: 3 additions & 0 deletions exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ func putfile(oeb *file.ObservableEditableBuffer, q0 int, q1 int, name string) er
if !os.SameFile(oeb.Info(), d) || d.ModTime().Sub(oeb.Info().ModTime()) > time.Millisecond {
oeb.UpdateInfo(name, d)
}

if !os.SameFile(oeb.Info(), d) || d.ModTime().Sub(oeb.Info().ModTime()) > time.Millisecond {
// By setting File.info here, a subsequent Put will ignore that
// the disk file was mutated and will write File to the disk file.
Expand Down Expand Up @@ -583,6 +584,8 @@ func putfile(oeb *file.ObservableEditableBuffer, q0 int, q1 int, name string) er
return nil
}

// TODO(rjk): Why doesn't this handle its arguments the same as some of
// the other commands?
func put(et *Text, _0 *Text, argt *Text, _1 bool, _2 bool, arg string) {
if et == nil || et.w == nil || et.w.body.file.IsDir() {
return
Expand Down
123 changes: 123 additions & 0 deletions exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,50 @@ func mutateBranchedAndRejoined(t *testing.T, g *globals) {
undo(&secondwin.tag, nil, nil, false /* this is not undo */, false /* ignored */, "")
}

func mutatePut(t *testing.T, g *globals) {
t.Helper()

firstwin := g.row.col[0].w[0]

// Mutate firstwin via cut.
firstwin.body.q0 = 3
firstwin.body.q1 = 10
global.seq++
firstwin.body.file.Mark(global.seq)
cut(&firstwin.tag, &firstwin.body, nil, false, true, "")

// Put the instance (This fails the first time because oeb.details.Info
// isn't set by the mock.)
put(&firstwin.tag, nil, nil, false, true, "")
put(&firstwin.tag, nil, nil, false, true, "")

// Validate that the file has the right contents.
fn := firstwin.body.file.Name()
contents, err := os.ReadFile(fn)
if err != nil {
t.Errorf("mutatePut can't read output file %v", err)
}

if got, want := string(contents), "Thishort text\nto try addressing\n"; got != want {
t.Errorf("mutatePut, put didn't succeed. got %q, want %q", got, want)
}
}

func mutatePutMutate(t *testing.T, g *globals) {
t.Helper()

mutatePut(t, g)

firstwin := g.row.col[0].w[0]

// Mutate firstwin via second cut.
firstwin.body.q0 = 0
firstwin.body.q1 = 4
global.seq++
firstwin.body.file.Mark(global.seq)
cut(&firstwin.tag, &firstwin.body, nil, false, true, "")
}

func TestUndoRedo(t *testing.T) {
dir := t.TempDir()
firstfilename, secondfilename := makeTempBackingFiles(t, dir)
Expand Down Expand Up @@ -617,6 +661,85 @@ func TestUndoRedo(t *testing.T) {
},
},
},
{
// Mutate, Put
name: "mutatePut",
fn: mutatePut,
want: &dumpfile.Content{
CurrentDir: cwd,
VarFont: defaultVarFont,
FixedFont: defaultFixedFont,
Columns: []dumpfile.Column{
{},
},
Windows: []*dumpfile.Window{
{
Type: dumpfile.Saved,
Column: 0,
Tag: dumpfile.Text{
Buffer: firstfilename + " Del Snarf Undo | Look Edit ",
},
Body: dumpfile.Text{
Buffer: "",
Q0: 3,
Q1: 3,
},
},
{
Type: dumpfile.Saved,
Column: 0,
Tag: dumpfile.Text{
Buffer: secondfilename + " Del Snarf | Look Edit ",
},
Body: dumpfile.Text{
Buffer: "",
Q0: 0,
Q1: 0,
},
},
},
},
},
{
// Mutate, Put, Mutate again.
// TODO(rjk): Undo sequence on top of this.
name: "mutatePutMutate",
fn: mutatePutMutate,
want: &dumpfile.Content{
CurrentDir: cwd,
VarFont: defaultVarFont,
FixedFont: defaultFixedFont,
Columns: []dumpfile.Column{
{},
},
Windows: []*dumpfile.Window{
{
Type: dumpfile.Unsaved,
Column: 0,
Tag: dumpfile.Text{
Buffer: firstfilename + " Del Snarf Undo Put | Look Edit ",
},
Body: dumpfile.Text{
Buffer: "hort text\nto try addressing\n",
Q0: 0,
Q1: 0,
},
},
{
Type: dumpfile.Saved,
Column: 0,
Tag: dumpfile.Text{
Buffer: secondfilename + " Del Snarf | Look Edit ",
},
Body: dumpfile.Text{
Buffer: "",
Q0: 0,
Q1: 0,
},
},
},
},
},
}

for _, tc := range tests {
Expand Down
11 changes: 10 additions & 1 deletion file/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,17 @@ func TestTagObserversFireCorrectly(t *testing.T) {
t.Errorf("got %+v, want %+v", got, want)
}

// Previous Clean makes the next observer fire so this one will invoke
// but since seq has not changed, it will not cause the last Clean to
// invoke the tag observers.
oeb.Clean()
if got, want := *counts, (observercount{0, 7}); got != want {
if got, want := *counts, (observercount{0, 8}); got != want {
t.Errorf("got %+v, want %+v", got, want)
}

// Last Clean
oeb.Clean()
if got, want := *counts, (observercount{0, 8}); got != want {
t.Errorf("got %+v, want %+v", got, want)
}
}
9 changes: 8 additions & 1 deletion file/observable_editable_buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,16 @@ func MakeObservableEditableBuffer(filename string, b []rune) *ObservableEditable
// TODO(rjk): Verify that this is invoked on a Put op.
func (e *ObservableEditableBuffer) Clean() {
before := e.getTagStatus()
defer e.notifyTagObservers(before)

e.treatasclean = false
op := e.putseq
e.putseq = e.seq

e.notifyTagObservers(before)

if op != e.seq {
e.filtertagobservers = false
}
}

// getTagState returns the current tag state. Assumption: this method needs to be cheap to
Expand Down Expand Up @@ -334,6 +340,7 @@ func (e *ObservableEditableBuffer) SetName(name string) {
// SetName always forces an update of the tag let the default tag update
// filter skip the name string comparison on the default case of editing
// a body.
// TODO(rjk): This reset of filtertagobservers might be unnecessary.
e.filtertagobservers = false
before := e.getTagStatus()
defer e.notifyTagObservers(before)
Expand Down
3 changes: 1 addition & 2 deletions wind.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,7 @@ func (w *Window) MemoizedUndone(undo bool) {
// log.Println("Window.MemoizedUndone")
}

// TODO(rjk): This should only get invokved when the tag needs to change
// do not double-invoke
func (w *Window) UpdateTag(newtagstatus file.TagStatus) {
// log.Printf("Window.UpdateTag, status %+v", newtagstatus)
w.setTag1()
}

0 comments on commit e48e054

Please sign in to comment.