From c90bed6f8ed0bbebf36a15dab786e0cae9e8a4ef Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 12 Apr 2020 01:36:53 +0200 Subject: [PATCH] Stop passing invalid coordinates to ExtendOrCreateCurrentBlock() This made the logic of this function unnecessarily more complicated. Instead, just fall back to the current cell coordinates in the only place where this could happen before. Doing this still preserves the correct behaviour of Shift-arrow selection when entire rows/columns are selected and the current cell is not the leftmost/topmost cell (due to scrolling), but the code is simpler. Remove the now always true condition check and assert that it's indeed always true. Note that the changes to gridsel.cpp in this commit are best viewed ignoring whitespace changes. --- include/wx/generic/gridsel.h | 7 +++-- src/generic/grid.cpp | 15 ++++++--- src/generic/gridsel.cpp | 60 ++++++++++++++++-------------------- 3 files changed, 41 insertions(+), 41 deletions(-) diff --git a/include/wx/generic/gridsel.h b/include/wx/generic/gridsel.h index cb53078f2c..1a64387c84 100644 --- a/include/wx/generic/gridsel.h +++ b/include/wx/generic/gridsel.h @@ -73,9 +73,10 @@ public: // wxGrid::m_currentCellCoords (the exception is when we scrolled out from // the top of the grid and select a column or scrolled right and select // a row: in this case the lowest visible row/column will be set as - // current, not the first one). If the row or the column component of - // blockEnd parameter has value of -1, it means that the corresponding - // component of the current block should not be changed. + // current, not the first one). + // + // Both components of both blockStart and blockEnd must be valid. + // // Return true if the current block was actually changed or created. bool ExtendOrCreateCurrentBlock(const wxGridCellCoords& blockStart, const wxGridCellCoords& blockEnd, diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index 56f1cfec6a..e8b0734dfa 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -7925,12 +7925,17 @@ bool wxGrid::PrepareForSelectionExpansion(wxGridCellCoords& coords, const wxGridDirectionOperations& diroper) { - coords.SetRow(m_selection->GetCurrentBlockCornerRow()); - coords.SetCol(m_selection->GetCurrentBlockCornerCol()); + int row = m_selection->GetCurrentBlockCornerRow(); + if ( row == -1 ) + row = m_currentCellCoords.GetRow(); - if ( coords == wxGridNoCellCoords ) - coords = m_currentCellCoords; - else if ( !diroper.IsValid(coords) ) + int col = m_selection->GetCurrentBlockCornerCol(); + if ( col == -1 ) + col = m_currentCellCoords.GetCol(); + + coords.Set(row, col); + + if ( !diroper.IsValid(coords) ) { // The component of the current block corner in our direction // is not valid. This means we can't change the selection block diff --git a/src/generic/gridsel.cpp b/src/generic/gridsel.cpp index cc361a0d97..963717aac0 100644 --- a/src/generic/gridsel.cpp +++ b/src/generic/gridsel.cpp @@ -486,6 +486,9 @@ bool wxGridSelection::ExtendOrCreateCurrentBlock(const wxGridCellCoords& blockSt const wxGridCellCoords& blockEnd, const wxKeyboardState& kbd) { + wxASSERT( blockStart.GetRow() != -1 && blockStart.GetCol() != -1 && + blockEnd.GetRow() != -1 && blockEnd.GetCol() != -1 ); + if ( m_selection.empty() ) { SelectBlock(blockStart, blockEnd); @@ -497,42 +500,34 @@ bool wxGridSelection::ExtendOrCreateCurrentBlock(const wxGridCellCoords& blockSt bool editBlock = false; - if ( blockEnd.GetRow() != -1 ) + // If the new block starts at the same top row as the current one, the + // end block coordinates must correspond to the new bottom row -- and + // vice versa, if the new block starts at the bottom, its other end + // must correspond to the top. + if ( blockStart.GetRow() == block.GetTopRow() ) { - // If the new block starts at the same top row as the current one, the - // end block coordinates must correspond to the new bottom row -- and - // vice versa, if the new block starts at the bottom, its other end - // must correspond to the top. - if ( blockStart.GetRow() == block.GetTopRow() ) - { - newBlock.SetBottomRow(blockEnd.GetRow()); - editBlock = true; - } - else if ( blockStart.GetRow() == block.GetBottomRow() ) - { - newBlock.SetTopRow(blockEnd.GetRow()); - editBlock = true; - } + newBlock.SetBottomRow(blockEnd.GetRow()); + editBlock = true; } - if ( blockEnd.GetCol() != -1 ) + else if ( blockStart.GetRow() == block.GetBottomRow() ) { - if ( newBlock.GetLeftCol() == blockStart.GetCol() ) - { - newBlock.SetRightCol(blockEnd.GetCol()); - editBlock = true; - } - else if ( newBlock.GetRightCol() == blockStart.GetCol() ) - { - newBlock.SetLeftCol(blockEnd.GetCol()); - editBlock = true; - } + newBlock.SetTopRow(blockEnd.GetRow()); + editBlock = true; + } + + if ( blockStart.GetCol() == block.GetLeftCol() ) + { + newBlock.SetRightCol(blockEnd.GetCol()); + editBlock = true; + } + else if ( blockStart.GetCol() == block.GetRightCol() ) + { + newBlock.SetLeftCol(blockEnd.GetCol()); + editBlock = true; } newBlock = newBlock.Canonicalize(); - const bool endCoordsSet = - blockEnd.GetRow() != -1 && blockEnd.GetCol() != -1; - if ( editBlock ) { if ( newBlock == block ) @@ -562,15 +557,14 @@ bool wxGridSelection::ExtendOrCreateCurrentBlock(const wxGridCellCoords& blockSt true, kbd); m_grid->GetEventHandler()->ProcessEvent(gridEvt); - return true; } - else if ( endCoordsSet ) + else { // Select the new one. SelectBlock(newBlock.GetTopLeft(), newBlock.GetBottomRight(), kbd); - return true; } - return false; + + return true; } int wxGridSelection::GetCurrentBlockCornerRow() const