cat / 012
clone

a spreadsheet for the terminal: lotus 1-2-3 looks, google sheets keys and formulas, one go binary

Regions in each other's way show the same whichever order their rows arrive in

Dami committed 1e7ce1d
A file reopened sends its regions their rows in another order than
they arrived in, so what regions meeting each other showed could differ
from what was saved. They now settle as arrays do (regionclash.go): a
region's anchor is in the way of every other region's table, and when
two tables need the same cells, the region whose anchor comes first,
row by row, gets them and the other gives way, showing why at its
anchor. A region blocked by another is sent again when the cells it
needs are freed or a region on its sheet goes. A region writing its
blocked anchor no longer takes a cell of the region it lies in.

Also:
- a region emptied (an array spilling over it, rows inserted before
  it, undo) recalculates the formulas naming it, which read #REF!
  until it's sent again, rather than keep the old sum;
- an array spilling over a region's table isn't blocked by a cycle
  through a formula naming the region, which reads #REF! once the
  region gives way;
- a paste into a blank cell of a region's table has the region find it
  in its way when sent again, as reopening does, rather than show
  around it.

TestRandomEdits sends stale outputs again until none is, at the start,
after each edit, undo, redo and reopening, as the UI does after every
change. Found with -randedit.seeds=4000 -randedit.steps=80 (seeds 1071,
3553, 3588, 3912) and -randedit.seeds=20000 (5067, 19178).
commit
1e7ce1dcf37e85c874c7e9b40ce7419224dd9793
parent
cbb411ffd26487c4c879c9063c2248f6831ba4c7
author
Dami <dami.etoile@gmail.com>
kind
fix risk score 2.1 of 3, Jev's estimate
12 files changed, +330 -62
Mdocs/contributing/architecture.md4+-
Mdocs/files/following.md5+-
Minternal/sheet/randedit_test.go26++++--
Minternal/sheet/recalc.go12++-
Minternal/sheet/region.go7+-
Ainternal/sheet/regionclash.go134++++++++++++++++++++++++++++++
Ainternal/sheet/regionclash_test.go103+++++++++++++++++++++++
Minternal/sheet/regionshape.go4+-
Minternal/sheet/regionwrite.go81+++++++++---------
Minternal/sheet/sheet.go2+-
Minternal/sheet/spillblock.go12++-
Minternal/sheet/tablebind.go2+-
M docs/contributing/architecture.md +3 -1
M docs/files/following.md +4 -1
M internal/sheet/randedit_test.go +19 -7
@@ -132,7 +132,7 @@ func randomEdits(data []byte, steps int) (string, []string) {
132132 break133133 }134134 log = append(log, r.edit(e))135- r.feedStale()135+ feedStale(r.wb, r.feeds)136136 }137137 end, endVals := r.save(), bookValues(r.wb)138138 if d := r.reopenDiff(end, endVals); d != "" {
@@ -144,6 +144,7 @@ func randomEdits(data []byte, steps int) (string, []string) {
144144 undone++145145 }146146 feedAll(r.wb, startFeeds)147+ feedStale(r.wb, startFeeds)147148 if got := r.save(); !bytes.Equal(got, start) {148149 return fmt.Sprintf("undoing %d steps saves differently:\n%s", undone, lineDiff(string(start), string(got))), log149150 }
@@ -154,6 +155,7 @@ func randomEdits(data []byte, steps int) (string, []string) {
154155 r.wb.Redo()155156 }156157 feedAll(r.wb, r.feeds)158+ feedStale(r.wb, r.feeds)157159 if got := r.save(); !bytes.Equal(got, end) {158160 return "redoing saves differently:\n" + lineDiff(string(end), string(got)), log159161 }
@@ -161,7 +163,7 @@ func randomEdits(data []byte, steps int) (string, []string) {
161163 return "redoing: " + d, log162164 }163165 r.wb.RecalcAll()164- r.feedStale()166+ feedStale(r.wb, r.feeds)165167 if d := diffValues(endVals, bookValues(r.wb)); d != "" {166168 return "recalculating everything: " + d, log167169 }
@@ -194,6 +196,7 @@ func newRandBook(e *edits) *randBook {
194196 wb.DefineName("Total", s, NewRect(Addr{}, Addr{Col: 1, Row: 9}))195197 nb.SetNotebookCells("add cells", []notebook.Cell{{Kind: notebook.Note, Source: "# Files"}, {Source: "r1 = ls"}})196198 r.addOutput(s2, "r1", e)199+ feedStale(wb, r.feeds)197200 wb.ClearHistory()198201 return r199202 }
@@ -232,11 +235,19 @@ func (r *randBook) feed(name string, e *edits) {
232235 }233236 234237 // feedStale sends the regions to be sent again (moved, put back by235-// undo) their source's rows, as the UI does after every change.236-func (r *randBook) feedStale() {237- for _, name := range r.wb.StaleOutputs() {238- if op, ok := r.feeds[name]; ok {239- sendRows(r.wb, op)238+// undo, no longer blocked) their source's rows, as the UI does after239+// every change, until none is: a region sent may free cells another240+// was blocked by, or take cells another gives way with.241+func feedStale(wb *Workbook, feeds map[string]LiveOp) {242+ for range 8 {243+ stale := wb.StaleOutputs()244+ if len(stale) == 0 {245+ return246+ }247+ for _, name := range stale {248+ if op, ok := feeds[name]; ok {249+ sendRows(wb, op)250+ }240251 }241252 }242253 }
@@ -282,6 +293,7 @@ func (r *randBook) reopenDiff(file []byte, vals map[string]cellValue) string {
282293 return "saves differently:\n" + lineDiff(string(file), string(again))283294 }284295 feedAll(wb, r.feeds)296+ feedStale(wb, r.feeds)285297 return diffValues(vals, bookValues(wb))286298 }287299
M internal/sheet/recalc.go +7 -5
M internal/sheet/region.go +4 -3
A internal/sheet/regionclash.go +134 -0
@@ -0,0 +1,134 @@
1+package sheet2+3+// Regions in each other's way (see region.go). What a region shows4+// can't depend on the order their rows arrived in, or a file would open5+// differently from how it was saved, its regions sent their rows in6+// another order. So, as for arrays (spill.go):7+//8+// - a region's anchor is in the way of every other region, whether9+// it shows a table, why it can't, or nothing yet, as an array's10+// formula is in the way of other arrays;11+// - when two tables need the same cells, the region whose anchor12+// comes first, row by row, gets them, whichever arrived first: the13+// other gives way, showing why at its anchor.14+//15+// A region blocked by another tries again, its source sending its rows,16+// when a cell it needs is freed, or a region on its sheet goes.17+18+// inTheWay returns a cell of need, the cells the region r with meta me19+// needs, holding something r didn't write and that keeps it from20+// showing, and the regions whose tables are to give way to it. Unless21+// whole is set, only what r didn't hold is looked at: nothing else is22+// placed in a region's cells without it knowing (regionTouched).23+func (s *Sheet) inTheWay(r Region, need Rect, me *regionMeta, whole bool) (at Addr, blocked bool, later []Region) {24+ // An array's cells are in the way even where it spilled blanks, as25+ // another array's are (spill.go).26+ for a, sp := range s.spills {27+ if x, ok := intersectRect(sp.area, need); ok && sp.why == "" {28+ if x.From != a {29+ return x.From, true, nil30+ }31+ return a, true, nil32+ }33+ }34+ for _, q := range s.regions.list {35+ if q.Name != r.Name && need.Contains(q.At) {36+ return q.At, true, nil37+ }38+ }39+ parts := []Rect{need}40+ if !whole {41+ parts = fresh(need, me)42+ }43+ for _, part := range parts {44+ if at, blocked = s.filledIn(part, r, me, &later); blocked {45+ return at, true, nil46+ }47+ }48+ return Addr{}, false, later49+}50+51+// filledIn returns a cell of part holding something the region r with52+// meta me didn't write that keeps it from showing, adding to later the53+// regions whose cells there come after r's anchor.54+func (s *Sheet) filledIn(part Rect, r Region, me *regionMeta, later *[]Region) (Addr, bool) {55+ for a := range s.cells.anyKeysIn(part) {56+ if !s.cells.filledAt(a) || s.regionOwns(a, me) {57+ continue58+ }59+ q, _, ok := s.ownerOf(a)60+ if !ok || before(q.At, r.At) {61+ return a, true62+ }63+ if !regionListed(*later, q) {64+ *later = append(*later, q)65+ }66+ }67+ return Addr{}, false68+}69+70+// before reports whether the cell at a comes before the cell at b, row71+// by row.72+func before(a, b Addr) bool { return a.Row < b.Row || a.Row == b.Row && a.Col < b.Col }73+74+func regionListed(list []Region, r Region) bool {75+ for _, x := range list {76+ if x.Name == r.Name {77+ return true78+ }79+ }80+ return false81+}82+83+// makeRoomAt has the regions whose tables hold r's anchor give way to84+// it, and returns the cells that changed.85+func (s *Sheet) makeRoomAt(r Region) []loc {86+ var changed []loc87+ for _, q := range s.regions.list {88+ if me := s.regionMeta[nameKey(q.Name)]; q.Name != r.Name && me != nil && me.why == "" && s.regionOwns(r.At, me) {89+ changed = append(changed, s.giveWay(q, Rect{From: r.At, To: r.At})...)90+ }91+ }92+ return changed93+}94+95+// giveWay has the region q, showing a table, show why it can't instead:96+// another region needs cells of it in taken. It returns the cells that97+// changed, with the formulas naming q.98+func (s *Sheet) giveWay(q Region, taken Rect) []loc {99+ me := s.meta(nameKey(q.Name))100+ at, _ := intersectRect(me.written, taken)101+ need, anchor := me.written, Rect{From: q.At, To: q.At}102+ changed := s.clearOwned(me, anchor)103+ me.why = "can't show: it would overwrite data in " + at.From.String()104+ me.need, me.written, me.rows, me.cols = need, anchor, 0, 0105+ changed = s.writeRegionCell(q.At, LiveCell{V: ErrRef}, me, changed)106+ return s.regionReaders(q, changed)107+}108+109+// wakeRegions marks the regions other than me blocked from showing by a110+// cell of gone, which me has just cleared, to be sent again.111+func (s *Sheet) wakeRegions(me *regionMeta, gone []Addr) {112+ for _, other := range s.regionMeta {113+ if other == me || other.why == "" {114+ continue115+ }116+ for _, a := range gone {117+ if other.need.Contains(a) {118+ other.stale = true119+ break120+ }121+ }122+ }123+}124+125+// wakeAllRegions marks every region of the sheet blocked from showing126+// to be sent again, as when a region goes: its anchor, which may have127+// blocked them, goes with it.128+func (s *Sheet) wakeAllRegions() {129+ for _, me := range s.regionMeta {130+ if me.why != "" {131+ me.stale = true132+ }133+ }134+}
A internal/sheet/regionclash_test.go +103 -0
@@ -0,0 +1,103 @@
1+package sheet2+3+import "testing"4+5+// Regions meeting each other show the same whichever order their rows6+// arrive in, as a file reopened sends them in another order than they7+// arrived in (regionclash.go).8+9+// sendBoth is a sheet with outputs a and b at the cells aAt and bAt,10+// sent opA and opB in that order, or the other with bFirst.11+func sendBoth(t *testing.T, aAt, bAt string, opA, opB LiveOp, bFirst bool) *Sheet {12+ t.Helper()13+ s := New()14+ sent(t, s, "a", aAt)15+ sent(t, s, "b", bAt)16+ s.Set(at("H1"), "=SUM(nu.a)")17+ s.Set(at("H2"), "=SUM(nu.b)")18+ ops := []LiveOp{opA, opB}19+ if bFirst {20+ ops = []LiveOp{opB, opA}21+ }22+ for _, op := range ops {23+ apply(t, s, op)24+ }25+ return s26+}27+28+// A region's anchor is in the way of another's table, whether or not29+// its own rows have arrived.30+func TestRegionAnchorBlocksAnother(t *testing.T) {31+ opA := LiveOp{Region: "a", Reset: true, Header: liveRow("x", "y"), Rows: []LiveRow{liveRow("1", "2")}}32+ opB := LiveOp{Region: "b", Reset: true, Header: liveRow("p", "q"), Rows: []LiveRow{liveRow("3", "4")}}33+ for _, bFirst := range []bool{false, true} {34+ s := sendBoth(t, "A1", "B2", opA, opB, bFirst)35+ wantShown(t, s, map[string]string{"A1": "#REF!", "A2": "", "B2": "p", "C3": "4", "H1": "#REF!", "H2": "7"})36+ }37+}38+39+// When two tables need the same cells, the region whose anchor comes40+// first, row by row, gets them, whichever arrived first.41+func TestRegionFirstAnchorWins(t *testing.T) {42+ // a needs B1:C3 and b A2:C2: neither holds the other's anchor.43+ opA := LiveOp{Region: "a", Reset: true, Header: liveRow("x", "y"), Rows: []LiveRow{liveRow("1", "2"), liveRow("3", "4")}}44+ opB := LiveOp{Region: "b", Reset: true, Header: liveRow("5", "6", "7")}45+ for _, bFirst := range []bool{false, true} {46+ s := sendBoth(t, "B1", "A2", opA, opB, bFirst)47+ wantShown(t, s, map[string]string{"B1": "x", "B2": "1", "C3": "4", "A2": "#REF!", "H1": "10", "H2": "#REF!"})48+ }49+}50+51+// A region an array spills over gives way, and the formulas naming it52+// read #REF! until it's sent again.53+func TestRegionGivingWayToAnArray(t *testing.T) {54+ s := New()55+ sent(t, s, "a", "B1")56+ s.Set(at("H1"), "=SUM(nu.a)")57+ apply(t, s, LiveOp{Region: "a", Reset: true, Header: liveRow("x"), Rows: []LiveRow{liveRow("5")}})58+ wantShown(t, s, map[string]string{"H1": "5"})59+ s.Set(at("A1"), "=SEQUENCE(1,2)")60+ wantShown(t, s, map[string]string{"A1": "1", "B1": "2", "B2": "", "H1": "#REF!"})61+}62+63+// An array spilling over a region's table isn't blocked by a formula64+// naming the region that it reads: the region gives way, so the array65+// doesn't depend on it, whichever came first.66+func TestArrayOverRegionItReadsByName(t *testing.T) {67+ for _, arrayFirst := range []bool{false, true} {68+ s := New()69+ sent(t, s, "a", "B1")70+ s.Set(at("D5"), "=SUM(nu.a)")71+ op := LiveOp{Region: "a", Reset: true, Header: liveRow("x"), Rows: []LiveRow{liveRow("5")}}72+ if !arrayFirst {73+ apply(t, s, op)74+ }75+ s.Set(at("A1"), "=IF(ISERROR(D5),SEQUENCE(1,2),SEQUENCE(1,2))") // over B176+ if arrayFirst {77+ apply(t, s, op)78+ }79+ wantShown(t, s, map[string]string{"A1": "1", "B1": "2", "D5": "#REF!"})80+ if _, ok := s.SpillAnchor(at("B1")); !ok {81+ t.Errorf("array first %v: the array doesn't spill over the region", arrayFirst)82+ }83+ }84+}85+86+// A paste into a blank cell of a region's table has the region find it87+// in its way when its rows are sent again, as they are when the file is88+// opened.89+func TestPasteIntoARegionsBlankCell(t *testing.T) {90+ s := New()91+ sent(t, s, "a", "A1")92+ op := LiveOp{Region: "a", Reset: true, Header: liveRow("x", "y"), Rows: []LiveRow{liveRow("1", "")}}93+ apply(t, s, op)94+ s.Set(at("E1"), "9")95+ if _, err := s.Paste(s.Copy(Rect{From: at("E1"), To: at("E1")}), Rect{From: at("B2"), To: at("B2")}, false); err != nil {96+ t.Fatal(err)97+ }98+ if got := s.Book().StaleOutputs(); len(got) != 1 {99+ t.Fatalf("stale outputs %v, want a", got)100+ }101+ apply(t, s, op)102+ wantShown(t, s, map[string]string{"A1": "#REF!", "A2": "", "B2": "9"})103+}
M internal/sheet/regionshape.go +2 -2
M internal/sheet/regionwrite.go +41 -40
@@ -21,12 +21,19 @@ func (s *Sheet) meta(k string) *regionMeta {
2121 return me2222 }2323 24-// regionTouched notes that the cell at a is being placed: a region25-// blocked by it tries again, its source sending its rows again.26-func (s *Sheet) regionTouched(a Addr) {24+// regionTouched notes that c is being placed at a: a region blocked by25+// it tries again, and one whose table it lands in (a blank cell of it,26+// pasted over) finds it in its way, its source sending its rows again27+// either way.28+func (s *Sheet) regionTouched(a Addr, c *Cell) {2729 for _, r := range s.regions.list {28- if me := s.regionMeta[nameKey(r.Name)]; me != nil && me.why != "" && me.need.Contains(a) {30+ me := s.regionMeta[nameKey(r.Name)]31+ switch {32+ case me == nil:33+ case me.why != "" && me.need.Contains(a):2934 me.stale = true35+ case me.why == "" && me.has && me.written.Contains(a) && !c.Blank():36+ me.stale, me.placed = true, true3037 }3138 }3239 }
@@ -70,6 +77,7 @@ func (s *Sheet) writeRegions() []loc {
7077 if s.regionIndex(k) < 0 {7178 changed = append(changed, s.clearOwned(me, noRect)...)7279 delete(s.regionMeta, k)80+ s.wakeAllRegions()7381 for u := range s.wb.nameUsers[nameKey(regionPrefix+k)] {7482 changed = append(changed, u) // the name reads nothing now7583 }
@@ -78,16 +86,20 @@ func (s *Sheet) writeRegions() []loc {
7886 for _, r := range s.regions.list {7987 if me := s.meta(nameKey(r.Name)); me.reread {8088 me.reread = false81- changed = append(changed, s.emptyRegion(me)...)89+ changed = append(changed, s.emptyRegion(r, me)...)8290 }8391 }8492 return changed8593 }8694 87-// emptyRegion clears a region's cells, outside the undo history, to be88-// fed again whole, which it is marked stale for.89-func (s *Sheet) emptyRegion(me *regionMeta) []loc {95+// emptyRegion clears the region r's cells, outside the undo history,96+// to be fed again whole, which it is marked stale for. The formulas97+// naming it read #REF! until then, so they're among the cells returned.98+func (s *Sheet) emptyRegion(r Region, me *regionMeta) []loc {9099 changed := s.clearOwned(me, noRect)100+ if me.has {101+ changed = s.regionReaders(r, changed)102+ }91103 me.has, me.rows, me.cols, me.why = false, 0, 0, ""92104 me.data, me.stale = 0, true93105 return changed
@@ -104,16 +116,22 @@ func (s *Sheet) writeTable(r Region, me *regionMeta, header LiveRow, rows []Live
104116 cols, nrows := tableSize(me, header, rows, reset)105117 was, had, shape := me.written, me.has, [2]int{me.rows, me.cols}106118 need := s.tableArea(r, cols, nrows)107- me.why = ""119+ changed := s.makeRoomAt(r)120+ whole := me.why != "" || me.placed // it may not hold every cell it wrote121+ me.why, me.placed = "", false108122 switch {109123 case need.To.Row >= MaxRows || need.To.Col >= MaxCols:110124 me.why = "can't show: it would go past the edge of the sheet"111125 default:112- if at, ok := s.inTheWay(need, me); ok {126+ at, blocked, later := s.inTheWay(r, need, me, whole)127+ if blocked {113128 me.why = "can't show: it would overwrite data in " + at.String()129+ break130+ }131+ for _, q := range later {132+ changed = append(changed, s.giveWay(q, need)...)114133 }115134 }116- var changed []loc117135 if me.why != "" {118136 me.need, me.rows, me.cols = need, 0, 0119137 need, header, rows, cols, nrows = Rect{From: r.At, To: r.At}, nil, nil, 0, 0
@@ -143,10 +161,16 @@ func (s *Sheet) writeTable(r Region, me *regionMeta, header LiveRow, rows []Live
143161 s.freedFor(c.a)144162 }145163 if !had || was != me.written || shape != [2]int{me.rows, me.cols} { // moved, or its table changed shape146- for _, k := range []string{nameKey(r.FormulaName()), nameKey(r.Name)} {147- for u := range s.wb.nameUsers[k] {148- changed = append(changed, u)149- }164+ changed = s.regionReaders(r, changed)165+ }166+ return changed167+}168+169+// regionReaders adds the formulas naming the region r to changed.170+func (s *Sheet) regionReaders(r Region, changed []loc) []loc {171+ for _, k := range []string{nameKey(r.FormulaName()), nameKey(r.Name)} {172+ for u := range s.wb.nameUsers[k] {173+ changed = append(changed, u)150174 }151175 }152176 return changed
@@ -205,30 +229,6 @@ func (s *Sheet) writeRegionCell(a Addr, c LiveCell, me *regionMeta, changed []lo
205229 return changed206230 }207231 208-// inTheWay returns a cell of need holding something the region with209-// meta me didn't write. Only what the region didn't hold is looked at:210-// nothing else can be placed in a region's cells.211-func (s *Sheet) inTheWay(need Rect, me *regionMeta) (Addr, bool) {212- // An array's cells are in the way even where it spilled blanks, as213- // another array's are (spill.go).214- for a, sp := range s.spills {215- if x, ok := intersectRect(sp.area, need); ok && sp.why == "" {216- if x.From != a {217- return x.From, true218- }219- return a, true220- }221- }222- for _, part := range fresh(need, me) {223- for a := range s.cells.anyKeysIn(part) {224- if s.cells.filledAt(a) && !s.regionOwns(a, me) {225- return a, true226- }227- }228- }229- return Addr{}, false230-}231-232232 // regionCell reports whether a region holds the cell at a.233233 func (s *Sheet) regionCell(a Addr) bool {234234 if len(s.regions.list) == 0 {
@@ -274,7 +274,7 @@ func (s *Sheet) regionsGiveWay(area Rect) []loc {
274274 var changed []loc275275 for _, r := range s.regions.list {276276 if me := s.regionMeta[nameKey(r.Name)]; me != nil && me.has && overlaps(me.written, area) {277- changed = append(changed, s.emptyRegion(me)...)277+ changed = append(changed, s.emptyRegion(r, me)...)278278 }279279 }280280 return changed
@@ -309,5 +309,6 @@ func (s *Sheet) clearOwned(me *regionMeta, keep Rect) []loc {
309309 s.setDerived(a, s.cells.get(a).leftover())310310 changed[i] = loc{s, a}311311 }312+ s.wakeRegions(me, gone)312313 return changed313314 }
M internal/sheet/sheet.go +1 -1
M internal/sheet/spillblock.go +11 -1
M internal/sheet/tablebind.go +1 -1