diff --git a/src/Mod/Spreadsheet/App/PropertySheet.cpp b/src/Mod/Spreadsheet/App/PropertySheet.cpp index 4ec7464c6c..10643a2f5d 100644 --- a/src/Mod/Spreadsheet/App/PropertySheet.cpp +++ b/src/Mod/Spreadsheet/App/PropertySheet.cpp @@ -588,7 +588,17 @@ void PropertySheet::setSpans(CellAddress address, int rows, int columns) nonNullCellAt(address)->setSpans(rows, columns); } -void PropertySheet::clear(CellAddress address) +void PropertySheet::clearAlias(CellAddress address) +{ + // Remove alias if it exists + std::map::iterator j = aliasProp.find(address); + if (j != aliasProp.end()) { + revAliasProp.erase(j->second); + aliasProp.erase(j); + } +} + +void PropertySheet::clear(CellAddress address, bool toClearAlias) { std::map::iterator i = data.find(address); @@ -607,18 +617,24 @@ void PropertySheet::clear(CellAddress address) // Mark as dirty dirty.insert(i->first); - // Remove alias if it exists - std::map::iterator j = aliasProp.find(address); - if (j != aliasProp.end()) { - revAliasProp.erase(j->second); - aliasProp.erase(j); - } + if (toClearAlias) + clearAlias(address); // Erase from internal struct data.erase(i); signaller.tryInvoke(); } +void PropertySheet::moveAlias(CellAddress currPos, CellAddress newPos) +{ + std::map::iterator j = aliasProp.find(currPos); + if (j != aliasProp.end()) { + aliasProp[newPos] = j->second; + revAliasProp[j->second] = newPos; + aliasProp.erase(currPos); + } +} + void PropertySheet::moveCell(CellAddress currPos, CellAddress newPos, std::map & renames) { std::map::const_iterator i = data.find(currPos); @@ -626,8 +642,10 @@ void PropertySheet::moveCell(CellAddress currPos, CellAddress newPos, std::mapsecond; @@ -639,12 +657,6 @@ void PropertySheet::moveCell(CellAddress currPos, CellAddress newPos, std::mapgetAlias(alias)) { - owner->aliasRemoved(currPos, alias); - cell->setAlias(""); - } - // Remove from old removeDependencies(currPos); data.erase(currPos); @@ -664,9 +676,6 @@ void PropertySheet::moveCell(CellAddress currPos, CellAddress newPos, std::mapsetAlias(alias); - setDirty(newPos); renames[ObjectIdentifier(owner, currPos.toString())] = ObjectIdentifier(owner, newPos.toString()); @@ -689,6 +698,13 @@ void PropertySheet::insertRows(int row, int count) CellAddress(row, CellAddress::MAX_COLUMNS), count, 0); AtomicPropertyChange signaller(*this); + + // move all the aliases first so dependencies can be calculated correctly + for (std::vector::const_reverse_iterator i = keys.rbegin(); i != keys.rend(); ++i) { + if (i->row() >= row) + moveAlias(*i, CellAddress(i->row() + count, i->col())); + } + for (std::vector::const_reverse_iterator i = keys.rbegin(); i != keys.rend(); ++i) { std::map::iterator j = data.find(*i); @@ -740,6 +756,15 @@ void PropertySheet::removeRows(int row, int count) CellAddress(row + count - 1, CellAddress::MAX_COLUMNS), -count, 0); AtomicPropertyChange signaller(*this); + + // move all the aliases first so dependencies can be calculated correctly + for (std::vector::const_iterator i = keys.begin(); i != keys.end(); ++i) { + if (i->row() >= row && i->row() < row + count) + clearAlias(*i); + else if (i->row() >= row + count) + moveAlias(*i, CellAddress(i->row() - count, i->col())); + } + for (std::vector::const_iterator i = keys.begin(); i != keys.end(); ++i) { std::map::iterator j = data.find(*i); @@ -756,7 +781,7 @@ void PropertySheet::removeRows(int row, int count) } if (i->row() >= row && i->row() < row + count) - clear(*i); + clear(*i, false); // aliases were cleared earlier else if (i->row() >= row + count) moveCell(*i, CellAddress(i->row() - count, i->col()), renames); } @@ -781,6 +806,13 @@ void PropertySheet::insertColumns(int col, int count) CellAddress(CellAddress::MAX_ROWS, col), 0, count); AtomicPropertyChange signaller(*this); + + // move all the aliases first so dependencies can be calculated correctly + for (std::vector::const_reverse_iterator i = keys.rbegin(); i != keys.rend(); ++i) { + if (i->col() >= col) + moveAlias(*i, CellAddress(i->row(), i->col() + count)); + } + for (std::vector::const_reverse_iterator i = keys.rbegin(); i != keys.rend(); ++i) { std::map::iterator j = data.find(*i); @@ -832,6 +864,15 @@ void PropertySheet::removeColumns(int col, int count) CellAddress(CellAddress::MAX_ROWS, col + count - 1), 0, -count); AtomicPropertyChange signaller(*this); + + // move all the aliases first so dependencies can be calculated correctly + for (std::vector::const_iterator i = keys.begin(); i != keys.end(); ++i) { + if (i->col() >= col && i->col() < col + count) + clearAlias(*i); + else if (i->col() >= col + count) + moveAlias(*i, CellAddress(i->row(), i->col() - count)); + } + for (std::vector::const_iterator i = keys.begin(); i != keys.end(); ++i) { std::map::iterator j = data.find(*i); @@ -848,7 +889,7 @@ void PropertySheet::removeColumns(int col, int count) } if (i->col() >= col && i->col() < col + count) - clear(*i); + clear(*i, false); // aliases were cleared earlier else if (i->col() >= col + count) moveCell(*i, CellAddress(i->row(), i->col() - count), renames); } diff --git a/src/Mod/Spreadsheet/App/PropertySheet.h b/src/Mod/Spreadsheet/App/PropertySheet.h index 54844087e9..fa5327fe6d 100644 --- a/src/Mod/Spreadsheet/App/PropertySheet.h +++ b/src/Mod/Spreadsheet/App/PropertySheet.h @@ -98,7 +98,7 @@ public: void setSpans(App::CellAddress address, int rows, int columns); - void clear(App::CellAddress address); + void clear(App::CellAddress address, bool toClearAlias=true); void clear(); @@ -126,8 +126,6 @@ public: bool isDirty() const { return dirty.size() > 0; } - void moveCell(App::CellAddress currPos, App::CellAddress newPos, std::map &renames); - void pasteCells(const std::map &cells, int rowOffset, int colOffset); void insertRows(int row, int count); @@ -216,6 +214,12 @@ private: /*! Owner of this property */ Sheet * owner; + void clearAlias(App::CellAddress address); + + void moveAlias(App::CellAddress currPos, App::CellAddress newPos); + + void moveCell(App::CellAddress currPos, App::CellAddress newPos, std::map &renames); + /* * Cell dependency tracking */ diff --git a/src/Mod/Spreadsheet/TestSpreadsheet.py b/src/Mod/Spreadsheet/TestSpreadsheet.py index 0f347f973e..0b6cee4efd 100644 --- a/src/Mod/Spreadsheet/TestSpreadsheet.py +++ b/src/Mod/Spreadsheet/TestSpreadsheet.py @@ -1017,6 +1017,57 @@ class SpreadsheetCases(unittest.TestCase): self.doc.recompute() self.assertEqual(sheet.get('C1'), Units.Quantity('3 mm')) + def testInsertRowsAlias(self): + """ Regression test for issue 4429; insert rows to sheet with aliases""" + sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet') + sheet.set('A3', '1') + sheet.setAlias('A3', 'alias1') + sheet.set('A4', '=alias1 + 1') + sheet.setAlias('A4', 'alias2') + sheet.set('A5', '=alias2 + 1') + self.doc.recompute() + sheet.insertRows('1', 1) + self.doc.recompute() + self.assertEqual(sheet.A6, 3) + + def testInsertColumnsAlias(self): + """ Regression test for issue 4429; insert columns to sheet with aliases""" + sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet') + sheet.set('C1', '1') + sheet.setAlias('C1', 'alias1') + sheet.set('D1', '=alias1 + 1') + sheet.setAlias('D1', 'alias2') + sheet.set('E1', '=alias2 + 1') + self.doc.recompute() + sheet.insertColumns('A', 1) + self.doc.recompute() + self.assertEqual(sheet.F1, 3) + + def testRemoveRowsAlias(self): + """ Regression test for issue 4429; remove rows from sheet with aliases""" + sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet') + sheet.set('A3', '1') + sheet.setAlias('A3', 'alias1') + sheet.set('A5', '=alias1 + 1') + sheet.setAlias('A5', 'alias2') + sheet.set('A4', '=alias2 + 1') + self.doc.recompute() + sheet.removeRows('1', 1) + self.doc.recompute() + self.assertEqual(sheet.A3, 3) + + def testRemoveColumnsAlias(self): + """ Regression test for issue 4429; remove columns from sheet with aliases""" + sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet') + sheet.set('C1', '1') + sheet.setAlias('C1', 'alias1') + sheet.set('E1', '=alias1 + 1') + sheet.setAlias('E1', 'alias2') + sheet.set('D1', '=alias2 + 1') + self.doc.recompute() + sheet.removeColumns('A', 1) + self.doc.recompute() + self.assertEqual(sheet.C1, 3) def tearDown(self): #closing doc