View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0026943 | Patches | LCL | public | 2014-10-25 19:03 | 2014-11-10 11:01 |
Reporter | Assigned To | Bart Broersma | |||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Platform | x64 (with x86 compiler) | OS | Windows | ||
Product Version | 1.2.6 | ||||
Target Version | 1.4 | ||||
Summary | 0026943: TStringgrid: InsertColRow on a cleared Grid | ||||
Description | InsertRowWithValues seems to insert rows without data at least in some cases. See example program; press button several times & you'll see the vertical scrollbar appear, indicating rows have been added. However they're empty. Also occurs on current trunk, FPC trunk x86, Windows. Note: this was actually a test program for a more complicated case which I cannot reproduce. | ||||
Additional Information | Please let me know if you require .zip instead of .7z My peazip did not like zipping up multiple files from the explorer so I'd have to create one manually. | ||||
Tags | patch | ||||
Fixed in Revision | r46766, r46782, r46787 | ||||
LazTarget | 1.4 | ||||
Widgetset | Win32/Win64 | ||||
Attached Files |
|
2014-10-25 19:03 developer |
|
|
[Off Topic] > Please let me know if you require .zip instead of .7z > My peazip did not like zipping up multiple files from the explorer so I'd have > to create one manually. I have 7-zip on Windows, so that should not be a problem. You know Windows Explorer (WinME and up) has built in capabilities of creating a straigth .zip? [/Off Topic] |
|
grids.addcolrow.diff (1,492 bytes)
Index: lcl/grids.pas =================================================================== --- lcl/grids.pas (revision 46593) +++ lcl/grids.pas (working copy) @@ -5775,7 +5775,7 @@ procedure TCustomGrid.DoOPInsertColRow(IsColumn: boolean; index: integer); var - NewCol,NewRow: Integer; + NewCol,NewRow,OldC: Integer; begin if Index<0 then Index:=0; @@ -5785,17 +5785,34 @@ if IsColumn then begin if Index>ColCount-1 then Index := ColCount-1; + if Index<0 then + Index := 0; if columns.Enabled then begin - Columns.InsertColumn(ColumnIndexFromGridColumn(index)); + if Columns.Count > 0 then + Columns.InsertColumn(ColumnIndexFromGridColumn(index)) + else + Columns.Add; ColRowInserted(true, index); exit; end else begin - FCols.Insert(Index, pointer(-1)); - FGCache.AccumWidth.Insert(Index, nil); + if ColCount > 0 then begin + FCols.Insert(Index, pointer(-1)); + FGCache.AccumWidth.Insert(Index, nil); + end else begin + OldC := RowCount; //Setting ColCount can set RowCount to 1 if RowCount = 0 + ColCount := 1; + RowCount := OldC; + Index := 0; + end; end; end else begin + if RowCount > 0 then begin Frows.Insert(Index, pointer(-1)); FGCache.AccumHeight.Insert(Index, nil); + end else begin + RowCount := 1; + Index := 0; + end; end; ColRowInserted(IsColumn, index); VisualChange; |
|
Problem seems to be in TCustomGrid.DoOPInsertColRow If you perform this on a cleared grid, then somehow internals structures get out of sync. (Run attached demo in insrow.zip, to see what happens.) It is also strange that this procedure raises an exception in this case (cleared grid) if IsColumn = True, but not if isColumn = False. Attached patch seems to fix it, but maybe not the right way to do it. Basically when RowCount or ColCount = 0 it sets it to 1 if needed. The strange side-effect seems to be that when doing this for a Column, under cirtain circumstances, it may then also set RowCount to 1 if RowCount was 0. @Jesus: can you plese review the patch? |
|
|
|
I think we have two options here: 1) raise an exception if trying to insert rows in a grid without columns, or trying to insert columns in a grid without rows, or 2) if trying to insert rows in a grid without columns add one, or trying to insert columns in a grid without rows add one row. Either option is OK to me, but I tend to prefer option 1, because option 2 is letting the grid guessing and it could guess wrong. Feel free to commit or suggest a patch that implements either option. On the other hand, InsertRowWithValues is a higher level function that carry information about rows and columns to be added, this information should be used to setup the grid (done in r46766). |
|
Trying to insert a row when no columns are present, or a columns when no rows are present now raises an exception. Please close if OK. |
|
Doesn't seem fixed to me r46786, FPC trunk - the test program still inserts empty data and does not show any exceptions. Not much of a grid/GUI guy here so I may have missed something? |
|
|
|
|
|
|
|
|
|
Uploaded a modified sample application (insrow2.zip). Look at screenshots to see what it does (insrow-1.png, insrow-2.png, insrow-3.png: the last one uses InsertRowWithValues, and succeeds as expected, the other two raise exceptions). Lazarus 1.3 r46782 FPC 2.6.4 32-bit on Win7-64. |
|
Bart, the third button in your demo program still seems to insert empty rows instead of the expected 'col1', 'col2' if you press it multiple times?!? |
|
Yes, you are right. Clear saves (a.o.) RowCount in FGridPropBackup.RowCount, which then is restored when InsertRowWithValues sets ColCount -> TCustomGrid.InternalSetColCount(). Invalidating FGridPropBackup seems not the right thing to do, so I decided to reset RowCount to 1, if it was 0 before inserting. |
|
Please close if OK now. |
|
Thanks a lot, works now! |
Date Modified | Username | Field | Change |
---|---|---|---|
2014-10-25 19:03 |
|
New Issue | |
2014-10-25 19:03 |
|
Status | new => assigned |
2014-10-25 19:03 |
|
Assigned To | => Bart Broersma |
2014-10-25 19:03 |
|
File Added: insertrowwithvalues.7z | |
2014-10-26 13:37 | Bart Broersma | Note Added: 0078618 | |
2014-10-26 16:06 | Bart Broersma | File Added: grids.addcolrow.diff | |
2014-10-26 16:13 | Bart Broersma | Note Added: 0078628 | |
2014-10-26 16:15 | Bart Broersma | File Added: insrow.zip | |
2014-10-26 16:15 | Bart Broersma | Assigned To | Bart Broersma => Jesus Reyes |
2014-10-26 16:15 | Bart Broersma | Project | Lazarus => Patches |
2014-10-26 16:16 | Bart Broersma | Tag Attached: patch | |
2014-10-26 17:30 | Bart Broersma | Summary | Stringgrid InsertRowWithValues inserts rows without data => TStringgrid: InsertColRow on a cleared Grid |
2014-11-06 20:45 | Jesus Reyes | Note Added: 0078965 | |
2014-11-06 20:47 | Jesus Reyes | Assigned To | Jesus Reyes => Bart Broersma |
2014-11-08 01:41 | Bart Broersma | Fixed in Revision | => r46782 |
2014-11-08 01:41 | Bart Broersma | LazTarget | - => 1.4 |
2014-11-08 01:41 | Bart Broersma | Note Added: 0078994 | |
2014-11-08 01:41 | Bart Broersma | Status | assigned => resolved |
2014-11-08 01:41 | Bart Broersma | Resolution | open => fixed |
2014-11-08 01:41 | Bart Broersma | Target Version | => 1.4 |
2014-11-08 09:57 |
|
Note Added: 0079000 | |
2014-11-08 09:57 |
|
Status | resolved => assigned |
2014-11-08 09:57 |
|
Resolution | fixed => reopened |
2014-11-08 14:20 | Bart Broersma | File Added: insrow2.zip | |
2014-11-08 14:20 | Bart Broersma | File Added: insrow-1.png | |
2014-11-08 14:20 | Bart Broersma | File Added: insrow-2.png | |
2014-11-08 14:21 | Bart Broersma | File Added: insrow-3.png | |
2014-11-08 14:24 | Bart Broersma | Note Added: 0079009 | |
2014-11-08 14:24 | Bart Broersma | Status | assigned => feedback |
2014-11-08 17:04 |
|
Note Added: 0079012 | |
2014-11-08 17:04 |
|
Status | feedback => assigned |
2014-11-08 17:42 | Bart Broersma | Note Added: 0079014 | |
2014-11-08 17:42 | Bart Broersma | Assigned To | Bart Broersma => Jesus Reyes |
2014-11-08 18:01 | Bart Broersma | Note Edited: 0079014 | View Revisions |
2014-11-08 18:03 | Bart Broersma | Fixed in Revision | r46782 => r46766, r46782, r46787 |
2014-11-08 18:03 | Bart Broersma | Note Added: 0079015 | |
2014-11-08 18:03 | Bart Broersma | Status | assigned => resolved |
2014-11-08 18:03 | Bart Broersma | Resolution | reopened => fixed |
2014-11-08 18:03 | Bart Broersma | Assigned To | Jesus Reyes => Bart Broersma |
2014-11-08 18:03 | Bart Broersma | Status | resolved => assigned |
2014-11-08 18:03 | Bart Broersma | Status | assigned => resolved |
2014-11-10 11:01 |
|
Note Added: 0079062 | |
2014-11-10 11:01 |
|
Status | resolved => closed |