View Issue Details

IDProjectCategoryView StatusLast Update
0030556LazarusLCLpublic2019-06-09 09:06
ReporterK155LA3Assigned ToJesus Reyes 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformOSWindowsOS Version7
Product Version1.7 (SVN)Product BuildRev. 51630 
Target Version1.8Fixed in Version1.7 (SVN) 
Summary0030556: TStringGrid empty cells does not replace cells with data when copy and paste one empty cell or one column with empty cell.
DescriptionIf you copy one empty cell or one column with empty cell and paste it in cell with data, than cells does not replace cells with data.
Steps To Reproduce1 Place StringGrid to form.
2 Set in Options goEditing.
3 Run programm.
4 Fill some cell with any data.
5 Select one empty cell and press Ctrl+C.
6 Place cursor on cell with data and press Ctrl+V
7 Nothing happens.
8 Select one column with empty cell and press Ctrl+C.
9 Place cursor on column with data and press Ctrl+V
10 If in the copied column there are empty cells, then it is not replaced the cell with data.
Additional InformationIn procedure TCustomStringGrid.SelectionSetText(TheText: String):

If copy one empty cell TheText = '' and after "L.Text := TheText" L.Count = 0 and cycle "for j:=0 to L.Count-1 do" not execute.

Because function "GetNextLine (Value,S,P)" in procedure "TStrings.SetTextStr(const Value: string) - > DoSetTextStr(Value,True) -> GetNextLine (Value,S,P)" when Value = '' do nothing.

If column with empty cell L[j] = '' and after "SubL.DelimitedText := L[j];" SubL.Count = 0 and cycle "for i:=0 to SubL.Count-1 do" not execute.
Because procedure "TStrings.SetDelimitedText(const AValue: string)" when AValue = '' do nothing.

May be it is problem of TStrings class. But now this bug for TCustomStringGrid may be fix with additional check:

procedure TCustomStringGrid.SelectionSetText(TheText: String);
...

    L.Text := TheText;
    if (TheText = '') and (L.Count = 0) then L.Add(''); //additional check (copy one empty cell)

...

      SubL.DelimitedText := L[j];
      if (L[j] = '') and (SubL.Count = 0) then SubL.Add(''); //additional check (copy one column with empty cell)
TagsNo tags attached.
Fixed in Revision52976
LazTarget1.8
Widgetset
Attached Files
  • TSGBug2.gif (102,306 bytes)
    TSGBug2.gif (102,306 bytes)
  • grids.pas.patch (1,200 bytes)
    Index: lcl/grids.pas
    ===================================================================
    --- lcl/grids.pas	(revision 52913)
    +++ lcl/grids.pas	(working copy)
    @@ -10471,6 +10471,28 @@
     var
       SelStr: String;
       i,j,k: LongInt;
    +
    +  function Quote(s: String): String;
    +  var
    +    i, j: Integer;
    +  begin
    +    SetLength(Result, Length(s) + 2);
    +    Result[1] := '"';
    +    i := 1;
    +    j := 2;
    +    while (i <= Length(s)) do begin
    +      Result[j] := s[i];
    +      if s[i] = '"' then begin
    +        SetLength(Result, Length(Result) + 1);
    +        inc(j);
    +        Result[j] := '"';
    +      end;
    +      inc(j);
    +      inc(i);
    +    end;
    +    Result[Length(Result)] := '"';
    +  end;
    +
     begin
       SelStr := '';
       for i:=R.Top to R.Bottom do begin
    @@ -10484,12 +10506,12 @@
               continue;
     
             if (i=0) then
    -          SelStr := SelStr + Columns[k].Title.Caption
    +          SelStr := SelStr + Quote(Columns[k].Title.Caption)
             else
    -          SelStr := SelStr + Cells[j,i];
    +          SelStr := SelStr + Quote(Cells[j,i]);
     
           end else
    -        SelStr := SelStr + Cells[j,i];
    +        SelStr := SelStr + Quote(Cells[j,i]);
     
           if j<>R.Right then
             SelStr := SelStr + #9;
    
    grids.pas.patch (1,200 bytes)

Relationships

related to 0030454 resolvedJesus Reyes TStringGrid copy selection bug, when cell in last column in selected range is empty. 
related to 0030607 resolvedJesus Reyes TStringGrid empty cells does not replace cells with data when copy and paste one empty cell 
related to 0030608 resolvedJesus Reyes TStringGrid when FixedRow = 0 instead of the data copied the title 

Activities

K155LA3

2016-09-04 08:43

reporter  

TSGBug2.gif (102,306 bytes)
TSGBug2.gif (102,306 bytes)

Bart Broersma

2016-09-04 13:10

developer   ~0094396

Please try with a revision >= r52904.

K155LA3

2016-09-04 17:40

reporter   ~0094407

Last edited: 2016-09-04 17:42

View 2 revisions

Sory, i'm set wrong revision of Lazarus 1.7(svn) in issue description.

I'm try revision 52913 from http://svn.freepascal.org/svn/lazarus/trunk/
Bug is present. Because TStrings does not set text through the procedures SetTextStr and SetDelimitedText when text is empty string.

SetTextStr('') -> DoSetTextStr('', ture) -> GetNextLine ('', S, 1)

Function GetNextLine('', S, 1) return False and cycle "While GetNextLine (Value,S,P) do Add(S)" not execute.

TStrings.SetDelimitedText(const AValue: string) when AValue = '' do nothing.
Cycle "while i<=length(AValue) do begin ..." does not execute because i = 1 and length(AValue) = 0.

I think this TStrings bug, SetTextStr and SetDelimitedText must be set empty string.

jamie philbrook

2016-09-04 21:59

reporter   ~0094410

It appears to be working as it should with any normal copy and paste operation.

 Blank entries carries blanks clipboard or string,. Normal.

 Use white spaces in blanks if you need this operation.

K155LA3

2016-09-05 18:20

reporter   ~0094427

> Blank entries carries blanks clipboard or string,. Normal.

And blank string must be replace string with data. For example S := '111' and after S := '' S will be a blank string. For this you does not use white spaces. And it is normal. The blank string it is the string with zero length.

If TStringGrid used copy/paste from clipboard, it is must be working correctly.

jamie philbrook

2016-09-06 00:41

reporter   ~0094431

Yes, for a spread sheet app that would be correct but, stringgrid isn't a
spread sheet and thus a feature has been added to make the Copy and Paste
work much like a spread sheet.
 I have expressed my opinion about a stringgrid having this feature without a
way to disable it. It causes issues with existing programs, like one I have that
uses cells with tabs and multiple lines within a cell. One must be careful not
to allow the block copy & paste to function.

 In your case however, you can populate your valid cells that become empty for
what ever reason with white spaces in the source, which is the standard method of filling in a blank with a blank that keeps it a non-empty cell.
 
 THe block operation can be corrected but I honestly believe that it should also be an option (block operation that is).

wp

2016-09-06 14:25

developer   ~0094436

Last edited: 2016-09-06 14:28

View 3 revisions

If a cell contains tabs and/or linebreaks it must be quoted before copying to clipbard. Care must be taken to consider the case that the cell could also contain quote characters. The StringGrids did not do this so far. But see attached patch "grid.pas.patch".

I absolutely disagree with your opinion that a copied blank cell should not erase a non-blank cell after pasting. That's against convention. A bank cell carries the information that it is empty, and the user wants to copy this information. Entering a space into a blank cell is an absolute no-go.

If you do not want to erase the receiving cell you must exclude the blank cell from the selection. In case of the TStringGrid you can use the RangeSelectMode rsmMulti for allowing multiple selections (using the CTRL key). Unfortunately this mode does not cooperate with the clipboard; some time ago I had posted a patch to fix this, but Jesus had not liked it ;-) (http://bugs.freepascal.org/view.php?id=28755).

wp

2016-09-06 14:26

developer  

grids.pas.patch (1,200 bytes)
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 52913)
+++ lcl/grids.pas	(working copy)
@@ -10471,6 +10471,28 @@
 var
   SelStr: String;
   i,j,k: LongInt;
+
+  function Quote(s: String): String;
+  var
+    i, j: Integer;
+  begin
+    SetLength(Result, Length(s) + 2);
+    Result[1] := '"';
+    i := 1;
+    j := 2;
+    while (i <= Length(s)) do begin
+      Result[j] := s[i];
+      if s[i] = '"' then begin
+        SetLength(Result, Length(Result) + 1);
+        inc(j);
+        Result[j] := '"';
+      end;
+      inc(j);
+      inc(i);
+    end;
+    Result[Length(Result)] := '"';
+  end;
+
 begin
   SelStr := '';
   for i:=R.Top to R.Bottom do begin
@@ -10484,12 +10506,12 @@
           continue;
 
         if (i=0) then
-          SelStr := SelStr + Columns[k].Title.Caption
+          SelStr := SelStr + Quote(Columns[k].Title.Caption)
         else
-          SelStr := SelStr + Cells[j,i];
+          SelStr := SelStr + Quote(Cells[j,i]);
 
       end else
-        SelStr := SelStr + Cells[j,i];
+        SelStr := SelStr + Quote(Cells[j,i]);
 
       if j<>R.Right then
         SelStr := SelStr + #9;
grids.pas.patch (1,200 bytes)

jamie philbrook

2016-09-07 03:18

reporter   ~0094452

Ok, I see your bound to your will. that's ok I understand.
I managed to get an old app to work by using the OnKey events to kill the
current block copy code. Reason? I need to examine cell content, check some
flags, and make changes at times from the source cells before they go to the
destination cells or land on the clipboard. Blank cells are dealt with at that
time cause I need to fill them with data.

 If you don't want to employ property to disable the Block copy/paste operation
you have now, is it at least possible to have a OnCellCopy and OnCellPaste.

These events would carry the Acol,Arow; Bcol,BRow, "Self" value for source and Destination of GRID instances, because that is important. If only from sys
clipboard then source "Self" would be nil.

 The String value of the Cell in VAR form so it can be changed before being appied to the destination cell and maybe even an Accept parameter.
 Allow changes to the Copying value incase this is to be a value meant for other
apps to retrieve. In which case the OnCelCopy would need to first fetch the cell contents and then call the event so you can make changes if need before it puts it on the clipboard.
 For multiple cells, like you do now, you simply call these events for each cell.

 So ether a property to disable block functions or implement what I suggested above so that we can still do this? I really hate hacking the key events to
kill the current implementation.

Jesus Reyes

2016-09-07 22:49

developer   ~0094471

@wp: Your Quote function seems to duplicate AnsiQuotedStr (which is used by QuotedStr) from StrUtils

wp

2016-09-08 00:31

developer   ~0094472

Right...

Jesus Reyes

2016-09-15 22:49

developer   ~0094660

Last edited: 2016-09-15 22:52

View 2 revisions

Fixed.

I implemented a variation of what jamie philbrook proposed, the reason is that I was doing some testing of copy/paste between several applications and some indeed do not overwrite destination cells where source cell is empty. With the new support this can be implemented and of course you can do custom conversion/processing of the copied/pasted text. This is done through a new event called OnCellProcess, the handler will take this arguments:

Sender: The Grid origin of the event
aCol, aRow: Coordinates of the cell being processed.
processType: process type, this can be cpCopy if a "copy" operation is being performed or cpPaste for the "paste" operation.
var aValue: This is the text that was copied or pasted, can be modified if needed. For example when pasting an empty text to a cell and you want to preserve whatever the cell has, you can do this:

if processType=cpPaste then begin
  if aValue='' then
    aValue := grid.cells[aCol,aRow]
end;

Well, please test.

jamie philbrook

2016-09-16 01:42

reporter   ~0094662

Please consider Acol, Arow of both sides in the call, so that we know Where it comes from and where it is going.

 Also a flag to indicate if it is Clipboard only as source, this would mean it
is coming from an external app which has no sender as source.

etc.

Jesus Reyes

2016-09-16 20:12

developer   ~0094685

Every cell would need to carry his own source coordinates, that can be accomplished by using a custom clipboard format, in addition to the current support. It is in my opinion just too much for a generic grid, as it would be a seldom used feature.

I believe, You can implement what you want with the current support, even, there are plenty of ways that you can use that I think you will probably find the best suited to your application.

K155LA3

2016-09-16 22:43

reporter   ~0094687

I'm try rev. 52982:

If you copy one empty cell and paste it in cell with data, than empty cells does not replace cells with data.
See new issue 30607: http://bugs.freepascal.org/view.php?id=30607

If you copy some cell from first row when FixedRow = 0 and paste it, than first row data not copied, they will replaced by the title.
See new issue 30608: http://bugs.freepascal.org/view.php?id=30608

Issue History

Date Modified Username Field Change
2016-09-04 08:43 K155LA3 New Issue
2016-09-04 08:43 K155LA3 File Added: TSGBug2.gif
2016-09-04 13:10 Bart Broersma LazTarget => -
2016-09-04 13:10 Bart Broersma Note Added: 0094396
2016-09-04 13:10 Bart Broersma Status new => feedback
2016-09-04 13:10 Bart Broersma Relationship added related to 0030454
2016-09-04 17:40 K155LA3 Note Added: 0094407
2016-09-04 17:40 K155LA3 Status feedback => new
2016-09-04 17:42 K155LA3 Note Edited: 0094407 View Revisions
2016-09-04 21:59 jamie philbrook Note Added: 0094410
2016-09-05 18:20 K155LA3 Note Added: 0094427
2016-09-05 18:58 Jesus Reyes Assigned To => Jesus Reyes
2016-09-05 18:58 Jesus Reyes Status new => assigned
2016-09-06 00:41 jamie philbrook Note Added: 0094431
2016-09-06 14:25 wp Note Added: 0094436
2016-09-06 14:26 wp File Added: grids.pas.patch
2016-09-06 14:26 wp Note Edited: 0094436 View Revisions
2016-09-06 14:28 wp Note Edited: 0094436 View Revisions
2016-09-07 03:18 jamie philbrook Note Added: 0094452
2016-09-07 22:49 Jesus Reyes Note Added: 0094471
2016-09-08 00:31 wp Note Added: 0094472
2016-09-15 22:49 Jesus Reyes Fixed in Revision => 52976
2016-09-15 22:49 Jesus Reyes LazTarget - => 1.8
2016-09-15 22:49 Jesus Reyes Note Added: 0094660
2016-09-15 22:49 Jesus Reyes Status assigned => resolved
2016-09-15 22:49 Jesus Reyes Fixed in Version => 1.7 (SVN)
2016-09-15 22:49 Jesus Reyes Resolution open => fixed
2016-09-15 22:49 Jesus Reyes Target Version => 1.8
2016-09-15 22:52 Jesus Reyes Note Edited: 0094660 View Revisions
2016-09-16 01:42 jamie philbrook Note Added: 0094662
2016-09-16 20:12 Jesus Reyes Note Added: 0094685
2016-09-16 22:43 K155LA3 Note Added: 0094687
2016-12-18 20:17 Jesus Reyes Relationship added related to 0030607
2016-12-18 20:19 Jesus Reyes Relationship added related to 0030608