- Undoing puts back cells a region moved over or grew over; the region is emptied first and, sent again, finds them in its way. - Formulas naming a region recalculate when its table changes shape, blocked included, and when the region is gone. - An array's cells, blank ones too, are in a region's way; an array spills over a region's table, which gives way, whichever came first, as when the file opens; clearing an array's cells never clears a region's, and a region blocked by an array is sent again once the array leaves.
- commit
- 788cacb0d0ad02469ed5b8e54b62d075f3bbce4c
- parent
- 7f2c1d208a5509af0f69029cd6a40e5c98854f08
- author
- Dami <dami.etoile@gmail.com>
- kind
- fix risk score 2.2 of 3, Jev's estimate
| M | internal/ | 6 | +++ |
| M | internal/ | 35 | ++++++++++++++++ |
| M | internal/ | 66 | ++++++++++++++++++++++++++++++ |
| M | internal/ | 52 | +++++++++++++++++++++-- |
| M | internal/ | 21 | +++++++-- |
@@ -15,6 +15,12 @@ func (w *Workbook) restore(st *step) (inv *step, changed []loc) {
1515 inv.sheets = w.sheetList()1616 w.setSheets(st.sheets)1717 }18+ for s, r := range st.regions {19+ changed = append(changed, s.emptyMoved(r)...)20+ }21+ for s, img := range st.cells {22+ changed = append(changed, s.emptyUnder(img)...)23+ }1824 for s, img := range st.cells {1925 img.each(func(a Addr, c *Cell) {2026 inv.keep(s, a)@@ -264,3 +264,38 @@ func (s *Sheet) putRegionsBack(st regionState) {
264264 }265265 s.regions, s.regionsStale = st, true266266 }267+268+// emptyUnder empties the regions holding cells that undo puts contents269+// back in (what a region's table grew over once they left), so the270+// contents come back and the region, sent again, finds them in its way,271+// as before they left.272+func (s *Sheet) emptyUnder(img *image) []loc {273+ if len(s.regions.list) == 0 {274+ return nil275+ }276+ var changed []loc277+ img.each(func(a Addr, c *Cell) {278+ if c == nil || c.Blank() || c.Spilled() {279+ return280+ }281+ if _, me, ok := s.ownerOf(a); ok {282+ changed = append(changed, s.emptyRegion(me)...)283+ }284+ })285+ return changed286+}287+288+// emptyMoved empties the regions that putting st back moves or takes289+// away, outside the undo history, as moving them does, before undo puts290+// cells back: a cell a region holds keeps its value when placed, so291+// what undo brings back where a region moved to would be lost.292+func (s *Sheet) emptyMoved(st regionState) []loc {293+ var changed []loc294+ for _, r := range s.regions.list {295+ i := slices.IndexFunc(st.list, func(x Region) bool { return nameKey(x.Name) == nameKey(r.Name) })296+ if me := s.regionMeta[nameKey(r.Name)]; me != nil && (i < 0 || st.list[i] != r) {297+ changed = append(changed, s.emptyRegion(me)...)298+ }299+ }300+ return changed301+}@@ -49,6 +49,72 @@ func TestSentOutputFollowsRuns(t *testing.T) {
4949 }5050 }5151 52+// Undoing a deleted column puts back the cells a region moved over and53+// was sent again at, rather than keep the region's values there.54+func TestUndoPutsBackCellsARegionMovedOver(t *testing.T) {55+ s := New()56+ w := s.Book()57+ s.Set(at("D19"), "=SEQUENCE(2)")58+ sent(t, s, "r1", "E19")59+ op := LiveOp{Region: "r1", Reset: true, Header: liveRow("n"), Rows: []LiveRow{liveRow("3")}}60+ apply(t, s, op)61+ s.DeleteCols(1, 1)62+ apply(t, s, op) // sent again where it moved, as the UI does63+ wantShown(t, s, map[string]string{"C19": "1", "D19": "n"})64+ w.Undo()65+ apply(t, s, op)66+ wantShown(t, s, map[string]string{"D19": "1", "D20": "2", "E19": "n", "E20": "3"})67+}68+69+// A formula naming a region whose table is blocked reads #REF!, even70+// when the table it showed was only its header, where the region's71+// first cell stays.72+func TestFormulaNamingABlockedRegion(t *testing.T) {73+ s := New()74+ s.Set(at("D1"), "=SUM(nu.r1)")75+ s.Set(at("F12"), "5")76+ sent(t, s, "r1", "E12")77+ apply(t, s, LiveOp{Region: "r1", Reset: true, Header: liveRow("n")})78+ wantShown(t, s, map[string]string{"D1": "0"})79+ apply(t, s, LiveOp{Region: "r1", Reset: true, Header: liveRow("n", "m"), Rows: []LiveRow{liveRow("1", "2")}})80+ wantShown(t, s, map[string]string{"E12": "#REF!", "D1": "#REF!"})81+}82+83+// Undoing a move puts back the cell a region's table grew over once the84+// cell left, and the region, sent again, is blocked by it as before.85+func TestUndoPutsBackCellsARegionGrewOver(t *testing.T) {86+ s := New()87+ w := s.Book()88+ s.Set(at("F14"), "=1+1")89+ sent(t, s, "r1", "E13")90+ op := LiveOp{Region: "r1", Reset: true, Header: liveRow("n", "m"), Rows: []LiveRow{liveRow("1", "2")}}91+ apply(t, s, op)92+ wantShown(t, s, map[string]string{"E13": "#REF!", "F14": "2"})93+ if _, err := s.Move(rect("F14"), at("A1")); err != nil {94+ t.Fatal(err)95+ }96+ apply(t, s, op)97+ wantShown(t, s, map[string]string{"E13": "n", "F14": "2", "A1": "2"})98+ w.Undo()99+ apply(t, s, op)100+ wantShown(t, s, map[string]string{"E13": "#REF!", "F14": "2", "A1": ""})101+ if s.Cell(at("F14")).Input != "=1+1" {102+ t.Errorf("F14 = %q", s.Cell(at("F14")).Input)103+ }104+}105+106+// A region whose column is deleted is gone, and formulas naming it say107+// so, as after reopening.108+func TestFormulaNamingARegionDeleted(t *testing.T) {109+ s := New()110+ s.Set(at("D1"), "=SUM(nu.r1)")111+ sent(t, s, "r1", "H3")112+ apply(t, s, LiveOp{Region: "r1", Reset: true, Header: liveRow("n"), Rows: []LiveRow{liveRow("56")}})113+ wantShown(t, s, map[string]string{"D1": "56"})114+ s.DeleteCols(7, 2)115+ wantShown(t, s, map[string]string{"D1": "#NAME?"})116+}117+52118 func TestSentOutputNames(t *testing.T) {53119 s := New()54120 w := s.Book()@@ -62,13 +62,17 @@ func (w *Workbook) settleRegions() {
6262 }6363 6464 // writeRegions empties the regions that are to be fed again and clears65-// the cells of regions gone, returning the cells that changed.65+// the cells of regions gone, returning the cells that changed and the66+// formulas naming regions gone.6667 func (s *Sheet) writeRegions() []loc {6768 var changed []loc6869 for k, me := range s.regionMeta {6970 if s.regionIndex(k) < 0 {7071 changed = append(changed, s.clearOwned(me, noRect)...)7172 delete(s.regionMeta, k)73+ for u := range s.wb.nameUsers[nameKey(regionPrefix+k)] {74+ changed = append(changed, u) // the name reads nothing now75+ }7276 }7377 }7478 for _, r := range s.regions.list {@@ -94,10 +98,11 @@ func (s *Sheet) emptyRegion(me *regionMeta) []loc {
9498 // nil rows show already. It writes only the cells that differ and9599 // clears the region's cells it no longer needs; when a cell it needs96100 // holds something else, it shows only why. It returns the cells that97-// changed, with the formulas naming the region when its table moved.101+// changed, with the formulas naming the region when its table moved or102+// changed shape.98103 func (s *Sheet) writeTable(r Region, me *regionMeta, header LiveRow, rows []LiveRow, reset bool) []loc {99104 cols, nrows := tableSize(me, header, rows, reset)100- was, had := me.written, me.has105+ was, had, shape := me.written, me.has, [2]int{me.rows, me.cols}101106 need := s.tableArea(r, cols, nrows)102107 me.why = ""103108 switch {@@ -137,7 +142,7 @@ func (s *Sheet) writeTable(r Region, me *regionMeta, header LiveRow, rows []Live
137142 for _, c := range changed {138143 s.freedFor(c.a)139144 }140- if !had || was != me.written {145+ if !had || was != me.written || shape != [2]int{me.rows, me.cols} { // moved, or its table changed shape141146 for u := range s.wb.nameUsers[nameKey(r.FormulaName())] {142147 changed = append(changed, u)143148 }@@ -202,6 +207,16 @@ func (s *Sheet) writeRegionCell(a Addr, c LiveCell, me *regionMeta, changed []lo
202207 // meta me didn't write. Only what the region didn't hold is looked at:203208 // nothing else can be placed in a region's cells.204209 func (s *Sheet) inTheWay(need Rect, me *regionMeta) (Addr, bool) {210+ // An array's cells are in the way even where it spilled blanks, as211+ // another array's are (spill.go).212+ for a, sp := range s.spills {213+ if x, ok := intersectRect(sp.area, need); ok && sp.why == "" {214+ if x.From != a {215+ return x.From, true216+ }217+ return a, true218+ }219+ }205220 for _, part := range fresh(need, me) {206221 for a := range s.cells.anyKeysIn(part) {207222 if s.cells.filledAt(a) && !s.regionOwns(a, me) {@@ -212,6 +227,15 @@ func (s *Sheet) inTheWay(need Rect, me *regionMeta) (Addr, bool) {
212227 return Addr{}, false213228 }214229 230+// regionCell reports whether a region holds the cell at a.231+func (s *Sheet) regionCell(a Addr) bool {232+ if len(s.regions.list) == 0 {233+ return false234+ }235+ _, _, ok := s.ownerOf(a)236+ return ok237+}238+215239 // fresh is the parts of need the region with meta me doesn't hold: all216240 // of it, or when need grows from what it holds, the strips to its right217241 // and below, so rows arriving cost what they add.@@ -233,7 +257,25 @@ func fresh(need Rect, me *regionMeta) []Rect {
233257 // writable reports whether the cell at a is free for the region with234258 // meta me to write.235259 func (s *Sheet) writable(a Addr, me *regionMeta) bool {236- return !s.cells.filledAt(a) || s.regionOwns(a, me)260+ if s.regionOwns(a, me) {261+ return true262+ }263+ _, spilled := s.SpillAnchor(a) // an array's blank cell is its own264+ return !spilled && !s.cells.filledAt(a)265+}266+267+// regionsGiveWay empties the regions whose tables are in area, where an268+// array spills: an array spills over a region's table, whichever came269+// first, as it does when the file opens and arrays spill before regions270+// are sent their rows. Sent again, a region finds the array in its way.271+func (s *Sheet) regionsGiveWay(area Rect) []loc {272+ var changed []loc273+ for _, r := range s.regions.list {274+ if me := s.regionMeta[nameKey(r.Name)]; me != nil && me.has && overlaps(me.written, area) {275+ changed = append(changed, s.emptyRegion(me)...)276+ }277+ }278+ return changed237279 }238280 239281 // regionOwns reports whether the cell at a is one the region with meta@@ -163,6 +163,7 @@ func (s *Sheet) applySpill(a Addr, p pendingSpill) ([]loc, bool) {
163163 // blocked by this one.164164 changed = append(append(changed, s.dropSpill(b)...), loc{s, b})165165 }166+ changed = append(changed, s.regionsGiveWay(area)...)166167 if old != nil && old.why == "" {167168 changed = append(changed, s.clearSpilled(old.area, area)...)168169 }@@ -220,8 +221,8 @@ func (s *Sheet) spillArea(a Addr, arr *functions.Array) (area Rect, why string,
220221 if old != nil && old.why == "" && old.area.Contains(at) {221222 continue222223 }223- if _, ok := s.SpillAnchor(at); ok {224- continue // an array's, found above224+ if _, ok := s.SpillAnchor(at); ok || s.regionCell(at) {225+ continue // an array's, found above, or a region's, which gives way225226 }226227 }227228 return area, "Array result was not expanded because it would overwrite data in " + at.String(), nil@@ -290,7 +291,7 @@ func (s *Sheet) clearSpilled(area, keep Rect) []loc {
290291 var changed []loc291292 var gone []Addr292293 for at := range s.cells.anyKeysIn(area) {293- if s.cells.derivedAt(at) == slotSpill && !keep.Contains(at) {294+ if s.cells.derivedAt(at) == slotSpill && !keep.Contains(at) && !s.regionCell(at) {294295 gone = append(gone, at)295296 }296297 }@@ -303,16 +304,26 @@ func (s *Sheet) clearSpilled(area, keep Rect) []loc {
303304 304305 // wakeBlocked returns the anchors of arrays blocked from spilling over305306 // the cells of area outside keep, which an array leaving them freed, to306-// compute again. The cells it left may have been blank, which changing307+// compute again, and marks the regions blocked there to be sent again.308+// The cells it left may have been blank, which changing307309 // wouldn't find.308310 func (s *Sheet) wakeBlocked(area, keep Rect) []loc {309311 var out []loc312+ freed := func(r Rect) bool {313+ x, ok := intersectRect(r, area)314+ return ok && !(keep.Contains(x.From) && keep.Contains(x.To))315+ }310316 for b, sp := range s.spills {311- if x, ok := intersectRect(sp.area, area); ok && sp.why != "" && !(keep.Contains(x.From) && keep.Contains(x.To)) {317+ if sp.why != "" && freed(sp.area) {312318 sp.stale = true313319 out = append(out, loc{s, b})314320 }315321 }322+ for _, me := range s.regionMeta {323+ if me.why != "" && freed(me.need) {324+ me.stale = true // sent again, as a region blocked by a cell cleared is325+ }326+ }316327 return out317328 }318329