View Issue Details

IDProjectCategoryView StatusLast Update
0035561PatchesLCLpublic2019-05-23 22:16
ReporterAlexey Tor.Assigned ToJuha Manninen 
PrioritynormalSeverityminorReproducibilityN/A
Status resolvedResolutionreopened 
Product Version2.1 (SVN)Product Build 
Target VersionFixed in Version 
Summary0035561: Refactor for Grids: add IsColumnIndexValid, IsRowIndexValid
Descriptionpatch adds two simplest methods, which is nice to have.
it makes code more readable.

ps. I cannot compile IDE after this patch:
Compile package LCL 2.1: Exit code 1, Errors: 1, Warnings: 1
Warning: Recompiling WSGrids, checksum changed for /home/user/lazarus/lcl/units/x86_64-linux/grids.ppu
wsgrids.pp(13,30) Fatal: Can't find unit WSGrids used by Grids
(even after "Clean all")
TagsNo tags attached.
Fixed in Revisionr61218, r61273
LazTarget-
Widgetset
Attached Files
  • grid.diff (6,426 bytes)
    Index: lcl/grids.pas
    ===================================================================
    --- lcl/grids.pas	(revision 61162)
    +++ lcl/grids.pas	(working copy)
    @@ -419,6 +419,8 @@
           procedure InsertColRow(IsColumn: Boolean; Index: Integer);
           procedure DisposeCell(var P: PCellProps); virtual;
           procedure DisposeColRow(var p: PColRowProps); virtual;
    +      function IsColumnIndexValid(AIndex: Integer): boolean;
    +      function IsRowIndexValid(AIndex: Integer): boolean;
         public
           constructor Create;
           destructor Destroy; override;
    @@ -1090,6 +1092,8 @@
         procedure InvalidateFromCol(ACol: Integer);
         procedure InvalidateGrid;
         procedure InvalidateFocused;
    +    function IsColumnIndexValid(AIndex: Integer): boolean;
    +    function IsRowIndexValid(AIndex: Integer): boolean;
         function  GetIsCellTitle(aCol,aRow: Integer): boolean; virtual;
         function  GetIsCellSelected(aCol, aRow: Integer): boolean; virtual;
         function IsEmptyRow(ARow: Integer): Boolean;
    @@ -2550,6 +2554,16 @@
       Result:=FRows.Count;
     end;
     
    +function TCustomGrid.IsColumnIndexValid(AIndex: Integer): boolean; inline;
    +begin
    +  Result := (AIndex>=0) and (AIndex<ColCount);
    +end;
    +
    +function TCustomGrid.IsRowIndexValid(AIndex: Integer): boolean; inline;
    +begin
    +  Result := (AIndex>=0) and (AIndex<RowCount);
    +end;
    +
     function TCustomGrid.GetColWidths(Acol: Integer): Integer;
     var
       C: TGridColumn;
    @@ -2556,7 +2570,7 @@
     begin
       if not Columns.Enabled or (aCol<FixedCols) then
       begin
    -    if (aCol<ColCount) and (aCol>=0) then
    +    if IsColumnIndexValid(aCol) then
           Result:=FCols[aCol]
         else
           Result:=-1;
    @@ -3539,10 +3553,8 @@
               [aCol,aRow,FGCache.FixedHeight,CHeight, FGCache.FixedWidth, CWidth]);
       {$Endif}
     
    -  while (fTopLeft.x>=0) and
    -        (fTopLeft.x<ColCount)and
    -        (fTopLeft.y>=0) and
    -        (fTopLeft.y<RowCount) do
    +  while IsColumnIndexValid(fTopLeft.x) and
    +        IsRowIndexValid(fTopLeft.y) do
       begin
         RNew:=CellRect(aCol,aRow);
         if UseRightToLeftAlignment then begin
    @@ -3600,9 +3612,8 @@
     
         if ((XInc=0)and(YInc=0)) or // the cell is already visible
            ((FTopLeft.X=aCol)and(FTopLeft.Y=aRow)) or // the cell is visible by definition
    -       ((FTopLeft.X+XInc<0)or(FTopLeft.Y+Yinc<0)) or // topleft can't be lower 0
    -       ((FTopLeft.X+XInc>=ColCount)) or // leftmost column can't be equal/higher than colcount
    -       ((FTopLeft.Y+Yinc>=RowCount)) // topmost column can't be equal/higher than rowcount
    +       not IsColumnIndexValid(FTopLeft.X+XInc) or
    +       not IsRowIndexValid(FTopLeft.Y+YInc)
         then
           Break;
         Inc(FTopLeft.x, XInc);
    @@ -3886,7 +3897,7 @@
     procedure TCustomGrid.ResetHotCell;
     begin
       with FGCache do begin
    -    if HotCellPainted and (HotCell.x < ColCount) and (HotCell.y < RowCount) then
    +    if HotCellPainted and IsColumnIndexValid(HotCell.x) and IsRowIndexValid(HotCell.y) then
           InvalidateCell(HotCell.X, HotCell.Y);
         HotCell := Point(-1,-1);
         HotCellPainted := False;
    @@ -6075,12 +6086,12 @@
           // begin to count Cols from 0 but ...
           if Fisical and (Offset>FixedWidth-1) then begin
             Index := FTopLeft.X;  // In scrolled view, then begin from FTopLeft col
    -        if (Index>=0) and (Index<ColCount) then begin
    +        if IsColumnIndexValid(Index) then begin
               Offset:=Offset-FixedWidth+AccumWidth[Index];
               if GetSmoothScroll(SB_Horz) then
                 Offset:=Offset+TLColOff;
             end;
    -        if (Index<0) or (Index>=ColCount) or (Offset>GridWidth-1) then begin
    +        if not IsColumnIndexValid(Index) or (Offset>GridWidth-1) then begin
               if AllowOutboundEvents then
                 Index := ColCount-1
               else
    @@ -6091,7 +6102,7 @@
     
           while Offset > AccumWidth[Index]+GetColWidths(Index)-1 do begin
             Inc(Index);
    -        if Index>=ColCount then begin
    +        if not IsColumnIndexValid(Index) then begin
               if AllowOutBoundEvents then
                 Index := ColCount-1
               else
    @@ -6145,12 +6156,12 @@
       Result:=false;
       with FGCache do begin
         if IsCol then begin
    -      if (index<0) or (index>ColCount-1) then
    +      if not IsColumnIndexValid(Index) then
             exit;
           StartPos:=AccumWidth[index];
           Dim:=GetColWidths(index);
         end else begin
    -      if (index<0) or (index>RowCount-1) then
    +      if not IsRowIndexValid(Index) then
             exit;
           StartPos:=AccumHeight[index];
           Dim:= GetRowHeights(index);
    @@ -8050,7 +8061,7 @@
     // shouldn't raise an error whereas setting the Row or Col property it should.
     procedure TCustomGrid.CheckLimitsWithError(const aCol, aRow: Integer);
     begin
    -  if (aCol < 0) or (aRow < 0) or (aCol >= ColCount) or (aRow >= RowCount) then
    +  if not IsColumnIndexValid(aCol) or not IsRowIndexValid(aRow) then
         raise EGridException.Create(rsGridIndexOutOfRange);
     end;
     
    @@ -8312,7 +8323,7 @@
     var
       C: TGridColumn;
     begin
    -  Result:=(goEditing in options) and (ACol>=0) and (ACol<ColCount) and (RowCount>FixedRows);
    +  Result:=(goEditing in options) and IsColumnIndexValid(ACol) and (RowCount>FixedRows);
       if Result and Columns.Enabled then begin
         C:=ColumnFromGridColumn(ACol);
         Result:=(C<>nil) and (not C.ReadOnly);
    @@ -9926,7 +9937,7 @@
     begin
       // todo: Check range
       Result:=nil;
    -  if (Col<0) or (Row<0) or (Col>=ColCount) or (Row>=RowCount) then
    +  if not IsColumnIndexValid(Col) or not IsRowIndexValid(Row) then
         raise EGridException.CreateFmt(rsIndexOutOfRange, [Col, Row]);
       Result:=FCellArr[Col,Row];
     end;
    @@ -10034,6 +10045,16 @@
       end;
     end;
     
    +function TVirtualGrid.IsColumnIndexValid(AIndex: Integer): boolean; inline;
    +begin
    +  Result := (AIndex>=0) and (AIndex<ColCount);
    +end;
    +
    +function TVirtualGrid.IsRowIndexValid(AIndex: Integer): boolean; inline;
    +begin
    +  Result := (AIndex>=0) and (AIndex<RowCount);
    +end;
    +
     function TVirtualGrid.GetDefaultCell: PcellProps;
     begin
       New(Result);
    @@ -11538,7 +11559,8 @@
                 bCellData := not bTagEnd;
                 if bTagEnd then // table end cell tag </td>
                 begin
    -              if (bCol < ColCount) and (bRow < RowCount) then Cells[bCol, bRow] := ReplaceEntities(bCellStr);
    +              if IsColumnIndexValid(bCol) and IsRowIndexValid(bRow) then
    +                Cells[bCol, bRow] := ReplaceEntities(bCellStr);
                   bSelRect.Right := bCol;
                   Inc(bCol);
                   bCellStr := '';
    
    grid.diff (6,426 bytes)
  • grid2.diff (10,461 bytes)
    Index: lcl/grids.pas
    ===================================================================
    --- lcl/grids.pas	(revision 61199)
    +++ lcl/grids.pas	(working copy)
    @@ -419,6 +419,8 @@
           procedure InsertColRow(IsColumn: Boolean; Index: Integer);
           procedure DisposeCell(var P: PCellProps); virtual;
           procedure DisposeColRow(var p: PColRowProps); virtual;
    +      function IsColumnIndexValid(AIndex: Integer): boolean;
    +      function IsRowIndexValid(AIndex: Integer): boolean;
         public
           constructor Create;
           destructor Destroy; override;
    @@ -1090,6 +1092,8 @@
         procedure InvalidateFromCol(ACol: Integer);
         procedure InvalidateGrid;
         procedure InvalidateFocused;
    +    function IsColumnIndexValid(AIndex: Integer): boolean;
    +    function IsRowIndexValid(AIndex: Integer): boolean;
         function  GetIsCellTitle(aCol,aRow: Integer): boolean; virtual;
         function  GetIsCellSelected(aCol, aRow: Integer): boolean; virtual;
         function IsEmptyRow(ARow: Integer): Boolean;
    @@ -2143,7 +2147,7 @@
     
     function TCustomGrid.GetRowHeights(Arow: Integer): Integer;
     begin
    -  if (aRow<RowCount) and (aRow>=0) then
    +  if IsRowIndexValid(aRow) then
         Result:=FRows[aRow]
       else
         Result:=-1;
    @@ -2550,6 +2554,16 @@
       Result:=FRows.Count;
     end;
     
    +function TCustomGrid.IsColumnIndexValid(AIndex: Integer): boolean; inline;
    +begin
    +  Result := (AIndex>=0) and (AIndex<ColCount);
    +end;
    +
    +function TCustomGrid.IsRowIndexValid(AIndex: Integer): boolean; inline;
    +begin
    +  Result := (AIndex>=0) and (AIndex<RowCount);
    +end;
    +
     function TCustomGrid.GetColWidths(Acol: Integer): Integer;
     var
       C: TGridColumn;
    @@ -2556,7 +2570,7 @@
     begin
       if not Columns.Enabled or (aCol<FixedCols) then
       begin
    -    if (aCol<ColCount) and (aCol>=0) then
    +    if IsColumnIndexValid(aCol) then
           Result:=FCols[aCol]
         else
           Result:=-1;
    @@ -3539,10 +3553,8 @@
               [aCol,aRow,FGCache.FixedHeight,CHeight, FGCache.FixedWidth, CWidth]);
       {$Endif}
     
    -  while (fTopLeft.x>=0) and
    -        (fTopLeft.x<ColCount)and
    -        (fTopLeft.y>=0) and
    -        (fTopLeft.y<RowCount) do
    +  while IsColumnIndexValid(fTopLeft.x) and
    +        IsRowIndexValid(fTopLeft.y) do
       begin
         RNew:=CellRect(aCol,aRow);
         if UseRightToLeftAlignment then begin
    @@ -3600,9 +3612,8 @@
     
         if ((XInc=0)and(YInc=0)) or // the cell is already visible
            ((FTopLeft.X=aCol)and(FTopLeft.Y=aRow)) or // the cell is visible by definition
    -       ((FTopLeft.X+XInc<0)or(FTopLeft.Y+Yinc<0)) or // topleft can't be lower 0
    -       ((FTopLeft.X+XInc>=ColCount)) or // leftmost column can't be equal/higher than colcount
    -       ((FTopLeft.Y+Yinc>=RowCount)) // topmost column can't be equal/higher than rowcount
    +       not IsColumnIndexValid(FTopLeft.X+XInc) or
    +       not IsRowIndexValid(FTopLeft.Y+YInc)
         then
           Break;
         Inc(FTopLeft.x, XInc);
    @@ -3886,7 +3897,7 @@
     procedure TCustomGrid.ResetHotCell;
     begin
       with FGCache do begin
    -    if HotCellPainted and (HotCell.x < ColCount) and (HotCell.y < RowCount) then
    +    if HotCellPainted and IsColumnIndexValid(HotCell.x) and IsRowIndexValid(HotCell.y) then
           InvalidateCell(HotCell.X, HotCell.Y);
         HotCell := Point(-1,-1);
         HotCellPainted := False;
    @@ -5130,7 +5141,7 @@
       begin
         if not GetSmoothScroll(SB_Horz) then
         begin
    -      if (FGCache.MaxTopLeft.x>=0) and (FGCache.MaxTopLeft.x<=ColCount-1) then
    +      if IsColumnIndexValid(FGCache.MaxTopLeft.x) then
             HsbRange := FGCache.AccumWidth[FGCache.MaxTopLeft.x]+ClientWidth-FGCache.FixedWidth
         end else
         begin
    @@ -5142,7 +5153,7 @@
               Dec(HsbRange, ColWidths[ColCount-1]);
           end;
         end;
    -    if (FTopLeft.x>=0) and (FTopLeft.x<=ColCount-1) then
    +    if IsColumnIndexValid(FTopLeft.x) then
           HsbPos := FGCache.AccumWidth[FTopLeft.x]+FGCache.TLColOff-FGCache.FixedWidth;
       end;
     
    @@ -5152,7 +5163,7 @@
       begin
         if not GetSmoothScroll(SB_Vert) then
         begin
    -      if (FGCache.MaxTopLeft.y>=0) and (FGCache.MaxTopLeft.y<=RowCount-1)  then
    +      if IsRowIndexValid(FGCache.MaxTopLeft.y)  then
             VsbRange := FGCache.AccumHeight[FGCache.MaxTopLeft.y]+ClientHeight-FGCache.FixedHeight
         end else
         begin
    @@ -5164,7 +5175,7 @@
               Dec(VsbRange, RowHeights[RowCount-1]);
           end;
         end;
    -    if (FTopLeft.y>=0) and (FTopLeft.y<=RowCount-1) then
    +    if IsRowIndexValid(FTopLeft.y) then
           VsbPos := FGCache.AccumHeight[FTopLeft.y]+FGCache.TLRowOff-FGCache.FixedHeight;
       end;
     
    @@ -5289,8 +5300,8 @@
     
     procedure TCustomGrid.CheckIndex(IsColumn: Boolean; Index: Integer);
     begin
    -  if (IsColumn and ((Index<0) or (Index>ColCount-1))) or
    -     (not IsColumn and ((Index<0) or (Index>RowCount-1))) then
    +  if (IsColumn and not IsColumnIndexValid(Index)) or
    +     (not IsColumn and not IsRowIndexValid(Index)) then
         raise EGridException.Create(rsGridIndexOutOfRange);
     end;
     
    @@ -6064,12 +6075,12 @@
           // begin to count Cols from 0 but ...
           if Fisical and (Offset>FixedWidth-1) then begin
             Index := FTopLeft.X;  // In scrolled view, then begin from FTopLeft col
    -        if (Index>=0) and (Index<ColCount) then begin
    +        if IsColumnIndexValid(Index) then begin
               Offset:=Offset-FixedWidth+AccumWidth[Index];
               if GetSmoothScroll(SB_Horz) then
                 Offset:=Offset+TLColOff;
             end;
    -        if (Index<0) or (Index>=ColCount) or (Offset>GridWidth-1) then begin
    +        if not IsColumnIndexValid(Index) or (Offset>GridWidth-1) then begin
               if AllowOutboundEvents then
                 Index := ColCount-1
               else
    @@ -6080,7 +6091,7 @@
     
           while Offset > AccumWidth[Index]+GetColWidths(Index)-1 do begin
             Inc(Index);
    -        if Index>=ColCount then begin
    +        if not IsColumnIndexValid(Index) then begin
               if AllowOutBoundEvents then
                 Index := ColCount-1
               else
    @@ -6098,9 +6109,9 @@
           //DebugLn('TCustomGrid.OffsetToColRow ',DbgSName(Self),' Fisical=',dbgs(Fisical),' Offset=',dbgs(Offset),' FixedHeight=',dbgs(FixedHeight),' FTopLeft=',dbgs(FTopLeft),' RowCount=',dbgs(RowCount),' TLRowOff=',dbgs(TLRowOff));
           if Fisical and (Offset>FixedHeight-1) then begin
             Index:=FTopLeft.Y;
    -        if (Index>=0) and (Index<RowCount) then
    +        if IsRowIndexValid(Index) then
               Offset:=Offset-FixedHeight+AccumHeight[Index]+TLRowOff;
    -        if (Index<0) or (Index>=RowCount) or (Offset>GridHeight-1) then begin
    +        if not IsRowIndexValid(Index) or (Offset>GridHeight-1) then begin
               if AllowOutboundEvents then
                 Index := RowCount-1
               else
    @@ -6134,12 +6145,12 @@
       Result:=false;
       with FGCache do begin
         if IsCol then begin
    -      if (index<0) or (index>ColCount-1) then
    +      if not IsColumnIndexValid(Index) then
             exit;
           StartPos:=AccumWidth[index];
           Dim:=GetColWidths(index);
         end else begin
    -      if (index<0) or (index>RowCount-1) then
    +      if not IsRowIndexValid(Index) then
             exit;
           StartPos:=AccumHeight[index];
           Dim:= GetRowHeights(index);
    @@ -8045,7 +8056,7 @@
     // shouldn't raise an error whereas setting the Row or Col property it should.
     procedure TCustomGrid.CheckLimitsWithError(const aCol, aRow: Integer);
     begin
    -  if (aCol < 0) or (aRow < 0) or (aCol >= ColCount) or (aRow >= RowCount) then
    +  if not IsColumnIndexValid(aCol) or not IsRowIndexValid(aRow) then
         raise EGridException.Create(rsGridIndexOutOfRange);
     end;
     
    @@ -8309,7 +8320,7 @@
     var
       C: TGridColumn;
     begin
    -  Result:=(goEditing in options) and (ACol>=0) and (ACol<ColCount) and (RowCount>FixedRows);
    +  Result:=(goEditing in options) and IsColumnIndexValid(ACol) and (RowCount>FixedRows);
       if Result and Columns.Enabled then begin
         C:=ColumnFromGridColumn(ACol);
         Result:=(C<>nil) and (not C.ReadOnly);
    @@ -9484,7 +9495,7 @@
             for i:=1 to k do begin
               tmpPath := Path+'column'+IntToStr(i);
               j:=cfg.getValue(tmpPath+'/index',-1);
    -          if (j>=0)and(j<=ColCount-1) then begin
    +          if IsColumnIndexValid(j) then begin
                 ColWidths[j]:=cfg.getValue(tmpPath+'/width',-1);
                 doLoadColumn(self, nil, j, Cfg, Version, tmpPath);
               end;
    @@ -9495,7 +9506,7 @@
           k:=cfg.getValue(Path+'rowcount',0);
           for i:=1 to k do begin
             j:=cfg.getValue(Path+'row'+IntToStr(i)+'/index',-1);
    -        if (j>=0)and(j<=RowCount-1) then begin
    +        if IsRowIndexValid(j) then begin
               RowHeights[j]:=cfg.getValue(Path+'row'+IntToStr(i)+'/height',-1);
             end;
           end;
    @@ -9925,7 +9936,7 @@
     begin
       // todo: Check range
       Result:=nil;
    -  if (Col<0) or (Row<0) or (Col>=ColCount) or (Row>=RowCount) then
    +  if not IsColumnIndexValid(Col) or not IsRowIndexValid(Row) then
         raise EGridException.CreateFmt(rsIndexOutOfRange, [Col, Row]);
       Result:=FCellArr[Col,Row];
     end;
    @@ -10033,6 +10044,16 @@
       end;
     end;
     
    +function TVirtualGrid.IsColumnIndexValid(AIndex: Integer): boolean; inline;
    +begin
    +  Result := (AIndex>=0) and (AIndex<ColCount);
    +end;
    +
    +function TVirtualGrid.IsRowIndexValid(AIndex: Integer): boolean; inline;
    +begin
    +  Result := (AIndex>=0) and (AIndex<RowCount);
    +end;
    +
     function TVirtualGrid.GetDefaultCell: PcellProps;
     begin
       New(Result);
    @@ -11164,7 +11185,7 @@
       aLayout: TButtonLayout;
       imgList: TCustomImageList;
     begin
    -  if (aCol<0) or (aCol>ColCount-1) then
    +  if not IsColumnIndexValid(aCol) then
         Exit;
     
       GetTitleImageInfo(aCol, i, aLayout);
    @@ -11537,7 +11558,8 @@
                 bCellData := not bTagEnd;
                 if bTagEnd then // table end cell tag </td>
                 begin
    -              if (bCol < ColCount) and (bRow < RowCount) then Cells[bCol, bRow] := ReplaceEntities(bCellStr);
    +              if IsColumnIndexValid(bCol) and IsRowIndexValid(bRow) then
    +                Cells[bCol, bRow] := ReplaceEntities(bCellStr);
                   bSelRect.Right := bCol;
                   Inc(bCol);
                   bCellStr := '';
    @@ -11593,7 +11615,7 @@
           while k>0 do begin
             i:=cfg.GetValue('grid/content/cells/cell'+IntToStr(k)+'/column', -1);
             j:=cfg.GetValue('grid/content/cells/cell'+IntTostr(k)+'/row',-1);
    -        if (j>=0)and(j<=rowcount-1)and(i>=0)and(i<=Colcount-1) then
    +        if IsRowIndexValid(j) and IsColumnIndexValid(i) then
               Cells[i,j]:=UTF8Encode(cfg.GetValue('grid/content/cells/cell'+IntToStr(k)+'/text',''));
             Dec(k);
           end;
    
    grid2.diff (10,461 bytes)
  • var.diff (2,531 bytes)
    Index: lcl/grids.pas
    ===================================================================
    --- lcl/grids.pas	(revision 61218)
    +++ lcl/grids.pas	(working copy)
    @@ -1094,6 +1094,8 @@
         procedure InvalidateFocused;
         function  IsColumnIndexValid(AIndex: Integer): boolean; inline;
         function  IsRowIndexValid(AIndex: Integer): boolean; inline;
    +    function  IsColumnIndexVariable(AIndex: Integer): boolean; inline;
    +    function  IsRowIndexVariable(AIndex: Integer): boolean; inline;
         function  GetIsCellTitle(aCol,aRow: Integer): boolean; virtual;
         function  GetIsCellSelected(aCol, aRow: Integer): boolean; virtual;
         function  IsEmptyRow(ARow: Integer): Boolean;
    @@ -2564,6 +2566,16 @@
       Result := (AIndex>=0) and (AIndex<RowCount);
     end;
     
    +function TCustomGrid.IsColumnIndexVariable(AIndex: Integer): boolean;
    +begin
    +  Result := (AIndex>=FFixedCols) and (AIndex<ColCount);
    +end;
    +
    +function TCustomGrid.IsRowIndexVariable(AIndex: Integer): boolean;
    +begin
    +  Result := (AIndex>=FFixedRows) and (AIndex<RowCount);
    +end;
    +
     function TCustomGrid.GetColWidths(Acol: Integer): Integer;
     var
       C: TGridColumn;
    @@ -6161,13 +6173,13 @@
           Exit;
         end;
         if IsCol then begin
    -      if index>=FFixedCols then begin
    +      if IsColumnIndexVariable(Index) then begin
             StartPos:=StartPos-AccumWidth[FTopLeft.X] + FixedWidth;
             if GetSmoothScroll(SB_Horz) then
               StartPos := StartPos - TLColOff;
           end;
         end else begin
    -      if index>=FFixedRows then begin
    +      if IsRowIndexVariable(Index) then begin
             StartPos:=StartPos-AccumHeight[FTopLeft.Y] + FixedHeight;
             if GetSmoothScroll(SB_Vert) then
               StartPos := StartPos - TLRowOff;
    @@ -7785,8 +7797,8 @@
       SelOk:=SelectCell(NCol,NRow);
       Result:=False;
       while not SelOk do begin
    -    if  (NRow+RInc>RowCount-1)or(NRow+RInc<FFixedRows) or
    -        (NCol+CInc>ColCount-1)or(NCol+CInc<FFixedCols) then Exit;
    +    if not IsRowIndexVariable(NRow+RInc) or
    +       not IsColumnIndexVariable(NCol+CInc) then Exit;
         Inc(NCol, CInc);
         Inc(NRow, RInc);
         SelOk:=SelectCell(NCol, NRow);
    @@ -9523,8 +9535,8 @@
           end;
           i:=Cfg.GetValue('grid/position/col',-1);
           j:=Cfg.GetValue('grid/position/row',-1);
    -      if (i>=FFixedCols)and(i<=ColCount-1) and
    -         (j>=FFixedRows)and(j<=RowCount-1) then begin
    +      if IsColumnIndexVariable(i) and
    +         IsRowIndexVariable(j) then begin
             MoveExtend(false, i,j, True);
           end;
           if goRangeSelect in Options then begin
    
    var.diff (2,531 bytes)

Activities

Alexey Tor.

2019-05-11 12:24

reporter  

grid.diff (6,426 bytes)
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 61162)
+++ lcl/grids.pas	(working copy)
@@ -419,6 +419,8 @@
       procedure InsertColRow(IsColumn: Boolean; Index: Integer);
       procedure DisposeCell(var P: PCellProps); virtual;
       procedure DisposeColRow(var p: PColRowProps); virtual;
+      function IsColumnIndexValid(AIndex: Integer): boolean;
+      function IsRowIndexValid(AIndex: Integer): boolean;
     public
       constructor Create;
       destructor Destroy; override;
@@ -1090,6 +1092,8 @@
     procedure InvalidateFromCol(ACol: Integer);
     procedure InvalidateGrid;
     procedure InvalidateFocused;
+    function IsColumnIndexValid(AIndex: Integer): boolean;
+    function IsRowIndexValid(AIndex: Integer): boolean;
     function  GetIsCellTitle(aCol,aRow: Integer): boolean; virtual;
     function  GetIsCellSelected(aCol, aRow: Integer): boolean; virtual;
     function IsEmptyRow(ARow: Integer): Boolean;
@@ -2550,6 +2554,16 @@
   Result:=FRows.Count;
 end;
 
+function TCustomGrid.IsColumnIndexValid(AIndex: Integer): boolean; inline;
+begin
+  Result := (AIndex>=0) and (AIndex<ColCount);
+end;
+
+function TCustomGrid.IsRowIndexValid(AIndex: Integer): boolean; inline;
+begin
+  Result := (AIndex>=0) and (AIndex<RowCount);
+end;
+
 function TCustomGrid.GetColWidths(Acol: Integer): Integer;
 var
   C: TGridColumn;
@@ -2556,7 +2570,7 @@
 begin
   if not Columns.Enabled or (aCol<FixedCols) then
   begin
-    if (aCol<ColCount) and (aCol>=0) then
+    if IsColumnIndexValid(aCol) then
       Result:=FCols[aCol]
     else
       Result:=-1;
@@ -3539,10 +3553,8 @@
           [aCol,aRow,FGCache.FixedHeight,CHeight, FGCache.FixedWidth, CWidth]);
   {$Endif}
 
-  while (fTopLeft.x>=0) and
-        (fTopLeft.x<ColCount)and
-        (fTopLeft.y>=0) and
-        (fTopLeft.y<RowCount) do
+  while IsColumnIndexValid(fTopLeft.x) and
+        IsRowIndexValid(fTopLeft.y) do
   begin
     RNew:=CellRect(aCol,aRow);
     if UseRightToLeftAlignment then begin
@@ -3600,9 +3612,8 @@
 
     if ((XInc=0)and(YInc=0)) or // the cell is already visible
        ((FTopLeft.X=aCol)and(FTopLeft.Y=aRow)) or // the cell is visible by definition
-       ((FTopLeft.X+XInc<0)or(FTopLeft.Y+Yinc<0)) or // topleft can't be lower 0
-       ((FTopLeft.X+XInc>=ColCount)) or // leftmost column can't be equal/higher than colcount
-       ((FTopLeft.Y+Yinc>=RowCount)) // topmost column can't be equal/higher than rowcount
+       not IsColumnIndexValid(FTopLeft.X+XInc) or
+       not IsRowIndexValid(FTopLeft.Y+YInc)
     then
       Break;
     Inc(FTopLeft.x, XInc);
@@ -3886,7 +3897,7 @@
 procedure TCustomGrid.ResetHotCell;
 begin
   with FGCache do begin
-    if HotCellPainted and (HotCell.x < ColCount) and (HotCell.y < RowCount) then
+    if HotCellPainted and IsColumnIndexValid(HotCell.x) and IsRowIndexValid(HotCell.y) then
       InvalidateCell(HotCell.X, HotCell.Y);
     HotCell := Point(-1,-1);
     HotCellPainted := False;
@@ -6075,12 +6086,12 @@
       // begin to count Cols from 0 but ...
       if Fisical and (Offset>FixedWidth-1) then begin
         Index := FTopLeft.X;  // In scrolled view, then begin from FTopLeft col
-        if (Index>=0) and (Index<ColCount) then begin
+        if IsColumnIndexValid(Index) then begin
           Offset:=Offset-FixedWidth+AccumWidth[Index];
           if GetSmoothScroll(SB_Horz) then
             Offset:=Offset+TLColOff;
         end;
-        if (Index<0) or (Index>=ColCount) or (Offset>GridWidth-1) then begin
+        if not IsColumnIndexValid(Index) or (Offset>GridWidth-1) then begin
           if AllowOutboundEvents then
             Index := ColCount-1
           else
@@ -6091,7 +6102,7 @@
 
       while Offset > AccumWidth[Index]+GetColWidths(Index)-1 do begin
         Inc(Index);
-        if Index>=ColCount then begin
+        if not IsColumnIndexValid(Index) then begin
           if AllowOutBoundEvents then
             Index := ColCount-1
           else
@@ -6145,12 +6156,12 @@
   Result:=false;
   with FGCache do begin
     if IsCol then begin
-      if (index<0) or (index>ColCount-1) then
+      if not IsColumnIndexValid(Index) then
         exit;
       StartPos:=AccumWidth[index];
       Dim:=GetColWidths(index);
     end else begin
-      if (index<0) or (index>RowCount-1) then
+      if not IsRowIndexValid(Index) then
         exit;
       StartPos:=AccumHeight[index];
       Dim:= GetRowHeights(index);
@@ -8050,7 +8061,7 @@
 // shouldn't raise an error whereas setting the Row or Col property it should.
 procedure TCustomGrid.CheckLimitsWithError(const aCol, aRow: Integer);
 begin
-  if (aCol < 0) or (aRow < 0) or (aCol >= ColCount) or (aRow >= RowCount) then
+  if not IsColumnIndexValid(aCol) or not IsRowIndexValid(aRow) then
     raise EGridException.Create(rsGridIndexOutOfRange);
 end;
 
@@ -8312,7 +8323,7 @@
 var
   C: TGridColumn;
 begin
-  Result:=(goEditing in options) and (ACol>=0) and (ACol<ColCount) and (RowCount>FixedRows);
+  Result:=(goEditing in options) and IsColumnIndexValid(ACol) and (RowCount>FixedRows);
   if Result and Columns.Enabled then begin
     C:=ColumnFromGridColumn(ACol);
     Result:=(C<>nil) and (not C.ReadOnly);
@@ -9926,7 +9937,7 @@
 begin
   // todo: Check range
   Result:=nil;
-  if (Col<0) or (Row<0) or (Col>=ColCount) or (Row>=RowCount) then
+  if not IsColumnIndexValid(Col) or not IsRowIndexValid(Row) then
     raise EGridException.CreateFmt(rsIndexOutOfRange, [Col, Row]);
   Result:=FCellArr[Col,Row];
 end;
@@ -10034,6 +10045,16 @@
   end;
 end;
 
+function TVirtualGrid.IsColumnIndexValid(AIndex: Integer): boolean; inline;
+begin
+  Result := (AIndex>=0) and (AIndex<ColCount);
+end;
+
+function TVirtualGrid.IsRowIndexValid(AIndex: Integer): boolean; inline;
+begin
+  Result := (AIndex>=0) and (AIndex<RowCount);
+end;
+
 function TVirtualGrid.GetDefaultCell: PcellProps;
 begin
   New(Result);
@@ -11538,7 +11559,8 @@
             bCellData := not bTagEnd;
             if bTagEnd then // table end cell tag </td>
             begin
-              if (bCol < ColCount) and (bRow < RowCount) then Cells[bCol, bRow] := ReplaceEntities(bCellStr);
+              if IsColumnIndexValid(bCol) and IsRowIndexValid(bRow) then
+                Cells[bCol, bRow] := ReplaceEntities(bCellStr);
               bSelRect.Right := bCol;
               Inc(bCol);
               bCellStr := '';
grid.diff (6,426 bytes)

Alexey Tor.

2019-05-11 15:02

reporter  

grid2.diff (10,461 bytes)
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 61199)
+++ lcl/grids.pas	(working copy)
@@ -419,6 +419,8 @@
       procedure InsertColRow(IsColumn: Boolean; Index: Integer);
       procedure DisposeCell(var P: PCellProps); virtual;
       procedure DisposeColRow(var p: PColRowProps); virtual;
+      function IsColumnIndexValid(AIndex: Integer): boolean;
+      function IsRowIndexValid(AIndex: Integer): boolean;
     public
       constructor Create;
       destructor Destroy; override;
@@ -1090,6 +1092,8 @@
     procedure InvalidateFromCol(ACol: Integer);
     procedure InvalidateGrid;
     procedure InvalidateFocused;
+    function IsColumnIndexValid(AIndex: Integer): boolean;
+    function IsRowIndexValid(AIndex: Integer): boolean;
     function  GetIsCellTitle(aCol,aRow: Integer): boolean; virtual;
     function  GetIsCellSelected(aCol, aRow: Integer): boolean; virtual;
     function IsEmptyRow(ARow: Integer): Boolean;
@@ -2143,7 +2147,7 @@
 
 function TCustomGrid.GetRowHeights(Arow: Integer): Integer;
 begin
-  if (aRow<RowCount) and (aRow>=0) then
+  if IsRowIndexValid(aRow) then
     Result:=FRows[aRow]
   else
     Result:=-1;
@@ -2550,6 +2554,16 @@
   Result:=FRows.Count;
 end;
 
+function TCustomGrid.IsColumnIndexValid(AIndex: Integer): boolean; inline;
+begin
+  Result := (AIndex>=0) and (AIndex<ColCount);
+end;
+
+function TCustomGrid.IsRowIndexValid(AIndex: Integer): boolean; inline;
+begin
+  Result := (AIndex>=0) and (AIndex<RowCount);
+end;
+
 function TCustomGrid.GetColWidths(Acol: Integer): Integer;
 var
   C: TGridColumn;
@@ -2556,7 +2570,7 @@
 begin
   if not Columns.Enabled or (aCol<FixedCols) then
   begin
-    if (aCol<ColCount) and (aCol>=0) then
+    if IsColumnIndexValid(aCol) then
       Result:=FCols[aCol]
     else
       Result:=-1;
@@ -3539,10 +3553,8 @@
           [aCol,aRow,FGCache.FixedHeight,CHeight, FGCache.FixedWidth, CWidth]);
   {$Endif}
 
-  while (fTopLeft.x>=0) and
-        (fTopLeft.x<ColCount)and
-        (fTopLeft.y>=0) and
-        (fTopLeft.y<RowCount) do
+  while IsColumnIndexValid(fTopLeft.x) and
+        IsRowIndexValid(fTopLeft.y) do
   begin
     RNew:=CellRect(aCol,aRow);
     if UseRightToLeftAlignment then begin
@@ -3600,9 +3612,8 @@
 
     if ((XInc=0)and(YInc=0)) or // the cell is already visible
        ((FTopLeft.X=aCol)and(FTopLeft.Y=aRow)) or // the cell is visible by definition
-       ((FTopLeft.X+XInc<0)or(FTopLeft.Y+Yinc<0)) or // topleft can't be lower 0
-       ((FTopLeft.X+XInc>=ColCount)) or // leftmost column can't be equal/higher than colcount
-       ((FTopLeft.Y+Yinc>=RowCount)) // topmost column can't be equal/higher than rowcount
+       not IsColumnIndexValid(FTopLeft.X+XInc) or
+       not IsRowIndexValid(FTopLeft.Y+YInc)
     then
       Break;
     Inc(FTopLeft.x, XInc);
@@ -3886,7 +3897,7 @@
 procedure TCustomGrid.ResetHotCell;
 begin
   with FGCache do begin
-    if HotCellPainted and (HotCell.x < ColCount) and (HotCell.y < RowCount) then
+    if HotCellPainted and IsColumnIndexValid(HotCell.x) and IsRowIndexValid(HotCell.y) then
       InvalidateCell(HotCell.X, HotCell.Y);
     HotCell := Point(-1,-1);
     HotCellPainted := False;
@@ -5130,7 +5141,7 @@
   begin
     if not GetSmoothScroll(SB_Horz) then
     begin
-      if (FGCache.MaxTopLeft.x>=0) and (FGCache.MaxTopLeft.x<=ColCount-1) then
+      if IsColumnIndexValid(FGCache.MaxTopLeft.x) then
         HsbRange := FGCache.AccumWidth[FGCache.MaxTopLeft.x]+ClientWidth-FGCache.FixedWidth
     end else
     begin
@@ -5142,7 +5153,7 @@
           Dec(HsbRange, ColWidths[ColCount-1]);
       end;
     end;
-    if (FTopLeft.x>=0) and (FTopLeft.x<=ColCount-1) then
+    if IsColumnIndexValid(FTopLeft.x) then
       HsbPos := FGCache.AccumWidth[FTopLeft.x]+FGCache.TLColOff-FGCache.FixedWidth;
   end;
 
@@ -5152,7 +5163,7 @@
   begin
     if not GetSmoothScroll(SB_Vert) then
     begin
-      if (FGCache.MaxTopLeft.y>=0) and (FGCache.MaxTopLeft.y<=RowCount-1)  then
+      if IsRowIndexValid(FGCache.MaxTopLeft.y)  then
         VsbRange := FGCache.AccumHeight[FGCache.MaxTopLeft.y]+ClientHeight-FGCache.FixedHeight
     end else
     begin
@@ -5164,7 +5175,7 @@
           Dec(VsbRange, RowHeights[RowCount-1]);
       end;
     end;
-    if (FTopLeft.y>=0) and (FTopLeft.y<=RowCount-1) then
+    if IsRowIndexValid(FTopLeft.y) then
       VsbPos := FGCache.AccumHeight[FTopLeft.y]+FGCache.TLRowOff-FGCache.FixedHeight;
   end;
 
@@ -5289,8 +5300,8 @@
 
 procedure TCustomGrid.CheckIndex(IsColumn: Boolean; Index: Integer);
 begin
-  if (IsColumn and ((Index<0) or (Index>ColCount-1))) or
-     (not IsColumn and ((Index<0) or (Index>RowCount-1))) then
+  if (IsColumn and not IsColumnIndexValid(Index)) or
+     (not IsColumn and not IsRowIndexValid(Index)) then
     raise EGridException.Create(rsGridIndexOutOfRange);
 end;
 
@@ -6064,12 +6075,12 @@
       // begin to count Cols from 0 but ...
       if Fisical and (Offset>FixedWidth-1) then begin
         Index := FTopLeft.X;  // In scrolled view, then begin from FTopLeft col
-        if (Index>=0) and (Index<ColCount) then begin
+        if IsColumnIndexValid(Index) then begin
           Offset:=Offset-FixedWidth+AccumWidth[Index];
           if GetSmoothScroll(SB_Horz) then
             Offset:=Offset+TLColOff;
         end;
-        if (Index<0) or (Index>=ColCount) or (Offset>GridWidth-1) then begin
+        if not IsColumnIndexValid(Index) or (Offset>GridWidth-1) then begin
           if AllowOutboundEvents then
             Index := ColCount-1
           else
@@ -6080,7 +6091,7 @@
 
       while Offset > AccumWidth[Index]+GetColWidths(Index)-1 do begin
         Inc(Index);
-        if Index>=ColCount then begin
+        if not IsColumnIndexValid(Index) then begin
           if AllowOutBoundEvents then
             Index := ColCount-1
           else
@@ -6098,9 +6109,9 @@
       //DebugLn('TCustomGrid.OffsetToColRow ',DbgSName(Self),' Fisical=',dbgs(Fisical),' Offset=',dbgs(Offset),' FixedHeight=',dbgs(FixedHeight),' FTopLeft=',dbgs(FTopLeft),' RowCount=',dbgs(RowCount),' TLRowOff=',dbgs(TLRowOff));
       if Fisical and (Offset>FixedHeight-1) then begin
         Index:=FTopLeft.Y;
-        if (Index>=0) and (Index<RowCount) then
+        if IsRowIndexValid(Index) then
           Offset:=Offset-FixedHeight+AccumHeight[Index]+TLRowOff;
-        if (Index<0) or (Index>=RowCount) or (Offset>GridHeight-1) then begin
+        if not IsRowIndexValid(Index) or (Offset>GridHeight-1) then begin
           if AllowOutboundEvents then
             Index := RowCount-1
           else
@@ -6134,12 +6145,12 @@
   Result:=false;
   with FGCache do begin
     if IsCol then begin
-      if (index<0) or (index>ColCount-1) then
+      if not IsColumnIndexValid(Index) then
         exit;
       StartPos:=AccumWidth[index];
       Dim:=GetColWidths(index);
     end else begin
-      if (index<0) or (index>RowCount-1) then
+      if not IsRowIndexValid(Index) then
         exit;
       StartPos:=AccumHeight[index];
       Dim:= GetRowHeights(index);
@@ -8045,7 +8056,7 @@
 // shouldn't raise an error whereas setting the Row or Col property it should.
 procedure TCustomGrid.CheckLimitsWithError(const aCol, aRow: Integer);
 begin
-  if (aCol < 0) or (aRow < 0) or (aCol >= ColCount) or (aRow >= RowCount) then
+  if not IsColumnIndexValid(aCol) or not IsRowIndexValid(aRow) then
     raise EGridException.Create(rsGridIndexOutOfRange);
 end;
 
@@ -8309,7 +8320,7 @@
 var
   C: TGridColumn;
 begin
-  Result:=(goEditing in options) and (ACol>=0) and (ACol<ColCount) and (RowCount>FixedRows);
+  Result:=(goEditing in options) and IsColumnIndexValid(ACol) and (RowCount>FixedRows);
   if Result and Columns.Enabled then begin
     C:=ColumnFromGridColumn(ACol);
     Result:=(C<>nil) and (not C.ReadOnly);
@@ -9484,7 +9495,7 @@
         for i:=1 to k do begin
           tmpPath := Path+'column'+IntToStr(i);
           j:=cfg.getValue(tmpPath+'/index',-1);
-          if (j>=0)and(j<=ColCount-1) then begin
+          if IsColumnIndexValid(j) then begin
             ColWidths[j]:=cfg.getValue(tmpPath+'/width',-1);
             doLoadColumn(self, nil, j, Cfg, Version, tmpPath);
           end;
@@ -9495,7 +9506,7 @@
       k:=cfg.getValue(Path+'rowcount',0);
       for i:=1 to k do begin
         j:=cfg.getValue(Path+'row'+IntToStr(i)+'/index',-1);
-        if (j>=0)and(j<=RowCount-1) then begin
+        if IsRowIndexValid(j) then begin
           RowHeights[j]:=cfg.getValue(Path+'row'+IntToStr(i)+'/height',-1);
         end;
       end;
@@ -9925,7 +9936,7 @@
 begin
   // todo: Check range
   Result:=nil;
-  if (Col<0) or (Row<0) or (Col>=ColCount) or (Row>=RowCount) then
+  if not IsColumnIndexValid(Col) or not IsRowIndexValid(Row) then
     raise EGridException.CreateFmt(rsIndexOutOfRange, [Col, Row]);
   Result:=FCellArr[Col,Row];
 end;
@@ -10033,6 +10044,16 @@
   end;
 end;
 
+function TVirtualGrid.IsColumnIndexValid(AIndex: Integer): boolean; inline;
+begin
+  Result := (AIndex>=0) and (AIndex<ColCount);
+end;
+
+function TVirtualGrid.IsRowIndexValid(AIndex: Integer): boolean; inline;
+begin
+  Result := (AIndex>=0) and (AIndex<RowCount);
+end;
+
 function TVirtualGrid.GetDefaultCell: PcellProps;
 begin
   New(Result);
@@ -11164,7 +11185,7 @@
   aLayout: TButtonLayout;
   imgList: TCustomImageList;
 begin
-  if (aCol<0) or (aCol>ColCount-1) then
+  if not IsColumnIndexValid(aCol) then
     Exit;
 
   GetTitleImageInfo(aCol, i, aLayout);
@@ -11537,7 +11558,8 @@
             bCellData := not bTagEnd;
             if bTagEnd then // table end cell tag </td>
             begin
-              if (bCol < ColCount) and (bRow < RowCount) then Cells[bCol, bRow] := ReplaceEntities(bCellStr);
+              if IsColumnIndexValid(bCol) and IsRowIndexValid(bRow) then
+                Cells[bCol, bRow] := ReplaceEntities(bCellStr);
               bSelRect.Right := bCol;
               Inc(bCol);
               bCellStr := '';
@@ -11593,7 +11615,7 @@
       while k>0 do begin
         i:=cfg.GetValue('grid/content/cells/cell'+IntToStr(k)+'/column', -1);
         j:=cfg.GetValue('grid/content/cells/cell'+IntTostr(k)+'/row',-1);
-        if (j>=0)and(j<=rowcount-1)and(i>=0)and(i<=Colcount-1) then
+        if IsRowIndexValid(j) and IsColumnIndexValid(i) then
           Cells[i,j]:=UTF8Encode(cfg.GetValue('grid/content/cells/cell'+IntToStr(k)+'/text',''));
         Dec(k);
       end;
grid2.diff (10,461 bytes)

Juha Manninen

2019-05-13 13:09

developer   ~0116159

The "inline;" is usually in interface section, not implementation section for methods. I changed it.
Otherwise the 2. patch looks good and improves code readability. Applied, thanks.
Here is compiles and works OK.

Alexey Tor.

2019-05-13 13:41

reporter  

var.diff (2,531 bytes)
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 61218)
+++ lcl/grids.pas	(working copy)
@@ -1094,6 +1094,8 @@
     procedure InvalidateFocused;
     function  IsColumnIndexValid(AIndex: Integer): boolean; inline;
     function  IsRowIndexValid(AIndex: Integer): boolean; inline;
+    function  IsColumnIndexVariable(AIndex: Integer): boolean; inline;
+    function  IsRowIndexVariable(AIndex: Integer): boolean; inline;
     function  GetIsCellTitle(aCol,aRow: Integer): boolean; virtual;
     function  GetIsCellSelected(aCol, aRow: Integer): boolean; virtual;
     function  IsEmptyRow(ARow: Integer): Boolean;
@@ -2564,6 +2566,16 @@
   Result := (AIndex>=0) and (AIndex<RowCount);
 end;
 
+function TCustomGrid.IsColumnIndexVariable(AIndex: Integer): boolean;
+begin
+  Result := (AIndex>=FFixedCols) and (AIndex<ColCount);
+end;
+
+function TCustomGrid.IsRowIndexVariable(AIndex: Integer): boolean;
+begin
+  Result := (AIndex>=FFixedRows) and (AIndex<RowCount);
+end;
+
 function TCustomGrid.GetColWidths(Acol: Integer): Integer;
 var
   C: TGridColumn;
@@ -6161,13 +6173,13 @@
       Exit;
     end;
     if IsCol then begin
-      if index>=FFixedCols then begin
+      if IsColumnIndexVariable(Index) then begin
         StartPos:=StartPos-AccumWidth[FTopLeft.X] + FixedWidth;
         if GetSmoothScroll(SB_Horz) then
           StartPos := StartPos - TLColOff;
       end;
     end else begin
-      if index>=FFixedRows then begin
+      if IsRowIndexVariable(Index) then begin
         StartPos:=StartPos-AccumHeight[FTopLeft.Y] + FixedHeight;
         if GetSmoothScroll(SB_Vert) then
           StartPos := StartPos - TLRowOff;
@@ -7785,8 +7797,8 @@
   SelOk:=SelectCell(NCol,NRow);
   Result:=False;
   while not SelOk do begin
-    if  (NRow+RInc>RowCount-1)or(NRow+RInc<FFixedRows) or
-        (NCol+CInc>ColCount-1)or(NCol+CInc<FFixedCols) then Exit;
+    if not IsRowIndexVariable(NRow+RInc) or
+       not IsColumnIndexVariable(NCol+CInc) then Exit;
     Inc(NCol, CInc);
     Inc(NRow, RInc);
     SelOk:=SelectCell(NCol, NRow);
@@ -9523,8 +9535,8 @@
       end;
       i:=Cfg.GetValue('grid/position/col',-1);
       j:=Cfg.GetValue('grid/position/row',-1);
-      if (i>=FFixedCols)and(i<=ColCount-1) and
-         (j>=FFixedRows)and(j<=RowCount-1) then begin
+      if IsColumnIndexVariable(i) and
+         IsRowIndexVariable(j) then begin
         MoveExtend(false, i,j, True);
       end;
       if goRangeSelect in Options then begin
var.diff (2,531 bytes)

Alexey Tor.

2019-05-13 13:41

reporter   ~0116160

var.diff. Additional refactor: IsColumnIndexVariable.

Jesus Reyes

2019-05-23 00:35

developer   ~0116356

Last edited: 2019-05-23 00:50

View 2 revisions

Just Please be careful with this kind of changes where you change the semantics, in the original code there is only one bound checking and in the new there are two bound checking, this could break the out of bounds functionality (I hope that this was checked in every instance in the first patch), TBH I don't know if this is currently the case, but it is suspicious that the new code do not take that in count.

Alexey Tor.

2019-05-23 22:16

reporter   ~0116380

I saw all places before doing the change, should be ok.

Issue History

Date Modified Username Field Change
2019-05-11 12:24 Alexey Tor. New Issue
2019-05-11 12:24 Alexey Tor. File Added: grid.diff
2019-05-11 15:02 Alexey Tor. File Added: grid2.diff
2019-05-13 13:02 Juha Manninen Assigned To => Juha Manninen
2019-05-13 13:02 Juha Manninen Status new => assigned
2019-05-13 13:09 Juha Manninen Status assigned => resolved
2019-05-13 13:09 Juha Manninen Resolution open => fixed
2019-05-13 13:09 Juha Manninen Fixed in Revision => r61218
2019-05-13 13:09 Juha Manninen LazTarget => -
2019-05-13 13:09 Juha Manninen Note Added: 0116159
2019-05-13 13:41 Alexey Tor. File Added: var.diff
2019-05-13 13:41 Alexey Tor. Status resolved => assigned
2019-05-13 13:41 Alexey Tor. Resolution fixed => reopened
2019-05-13 13:41 Alexey Tor. Note Added: 0116160
2019-05-22 16:43 Juha Manninen Status assigned => resolved
2019-05-22 16:43 Juha Manninen Fixed in Revision r61218 => r61218, r61273
2019-05-23 00:35 Jesus Reyes Note Added: 0116356
2019-05-23 00:50 Jesus Reyes Note Edited: 0116356 View Revisions
2019-05-23 22:16 Alexey Tor. Note Added: 0116380