View Issue Details

IDProjectCategoryView StatusLast Update
0030454LazarusLCLpublic2019-06-09 09:07
ReporterK155LA3Assigned ToJesus Reyes 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformOSWindowsOS Version7
Product Version1.6Product Build 
Target Version1.8Fixed in Version1.7 (SVN) 
Summary0030454: TStringGrid copy selection bug, when cell in last column in selected range is empty.
DescriptionIf you copy multiple cells and the last will be empty, when you paste the copied range, the last empty cell is not inserted.

This bug is present in version 1.7(SVN).
Steps To Reproduce1 Place StringGrid to form.
2 Set in Options goEditing
3 Run programm
4 Fill some cell with any data.
5 Select some empty cell an press Ctrl+C
6 Place cursor on first cell with data and press Ctrl+V
7 All except the last cell will be replaced.
Additional InformationThis problem located in sub procedure "CollectCols" in procedure "TCustomStringGrid.SelectionSetText":

  begin
    Subl.Clear;
    P := Pchar(S);
    if P<>nil then
      while P^<>#0 do begin
        ini := P;
        while (P^<>#0) and (P^<>0000009) do
          Inc(P);
        if P=Ini then
          St := ''
        else begin
          SetLength(St, P-Ini);
          Move(Ini^,St[1],P-Ini);
        end;
        SubL.Add(St);
        if P^<>#0 then
          Inc(P);
      end;
  end;

If last cell was empty, than after checks "if P^<>#0 then Inc(P);" P^ will is #0 and cycle was end.

To eliminate such a need to introduce additional checks:

  begin
    Subl.Clear;
    P := Pchar(S);
    if P<>nil then
      while P^<>#0 do begin
        ini := P;
        while (P^<>#0) and (P^<>0000009) do
          Inc(P);
        if P=Ini then
          St := ''
        else begin
          SetLength(St, P-Ini);
          Move(Ini^,St[1],P-Ini);
        end;
        SubL.Add(St);
        if P^<>#0 then
        begin
          Inc(P);
          if P^ = #0 then SubL.Add(''); //additional checks
        end;
      end;
  end;
TagsNo tags attached.
Fixed in Revision52904
LazTarget1.8
WidgetsetWin32/Win64
Attached Files

Relationships

related to 0030608 resolvedJesus Reyes TStringGrid when FixedRow = 0 instead of the data copied the title 
related to 0030607 resolvedJesus Reyes TStringGrid empty cells does not replace cells with data when copy and paste one empty cell 
related to 0030556 resolvedJesus Reyes TStringGrid empty cells does not replace cells with data when copy and paste one empty cell or one column with empty cell. 
related to 0030623 resolvedJesus Reyes TStringGrid copy/paste to/from MS Excel and OO Calc bug 

Activities

K155LA3

2016-08-09 21:48

reporter  

TSGBug.gif (69,094 bytes)
TSGBug.gif (69,094 bytes)

K155LA3

2016-08-11 21:14

reporter   ~0094100

Found some additional bug:
If you copy one column contains blank cells in a column of filled cells, the filled cells are not will be replaced with blank.
Because an empty cell after P: = Pchar (S) give P ^ = # 0, respectively in line SubL will not be added and SubL.Count is 0, respectively cycle "for i: = 0 to SubL.Count-1 do ..." is not execute and the cell is not filled empty data.

Additional checks can solve this problem:

  procedure CollectCols(const S: String);
  var
    P,Ini: PChar;
    St: String;
  begin
    Subl.Clear;
    P := Pchar(S);
    if P<>nil then
      begin
        if P^ = #0 then SubL.Add(''); //additional checks for one empty cell
        while P^<>#0 do begin
          ini := P;
          while (P^<>#0) and (P^<>0000009) do
            Inc(P);
          if P=Ini then
            St := ''
          else begin
            SetLength(St, P-Ini);
            Move(Ini^,St[1],P-Ini);
          end;
          SubL.Add(St);
          if P^<>#0 then
          begin
            Inc(P);
            if P^ = #0 then SubL.Add(''); //additional checks for last empty cell
          end;
        end;
      end;
  end;

Jesus Reyes

2016-09-01 19:40

developer   ~0094374

Fixed. Now it's using TStrings DelimitedText functionality.

Please test.

K155LA3

2016-09-03 11:49

reporter   ~0094385

Last edited: 2016-09-04 09:06

View 3 revisions

I'm test it with trunk from https://github.com/alrieckert/lazarus
Copy selection with two or more column work fine, but when copy one empty cell or column with empty cells then empty cells not replaced cells with data.

In 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)

See new issue 30556: http://bugs.freepascal.org/view.php?id=30556

jamie philbrook

2016-09-04 22:14

reporter   ~0094411

Come on guys, you don't overwrite cells from empty ones.. that is not
how Copy C and V works. those are clipboard functions only. Empty content in
the source means no content to destination, which means you don't touch the
destination.

 I sure hope you plan on implementing a property to disable this feature, this
isn't going to fly with piles of code already out there.

Jesus Reyes

2016-09-05 19:28

developer   ~0094428

Last edited: 2016-09-05 19:29

View 2 revisions

Just a clarification, before this fix (I mean r52904), 'internal' empty cells were always overwriting destination content. The problem was in the cells on column border of selection because those cells were not overwriting destination content, this is what this patch has corrected. It remains the problem K155LA3 has reported. Hopefully it will be fixed soon.

As a reminder (or if I have not made that public I'm sorry but now it is), the copy and paste feature was implemented and should work the same way it does in spreadsheet programs like libre/open office, excel and gnumerics and should be able to interact with those programs. Also in those programs empty cells overwrite destination content.

@jamie: please submit a bug report with an example that demonstrates how this fix has broken it and also explain how a 'property to disable this feature' should work.

jamie philbrook

2016-09-06 00:21

reporter   ~0094430

anyway to disable that feature? this is a stringgrid not a spread sheet.
I tried porting over simple example from a D app, it made a mess out of it.
 
 standard stringgrid allows you to have multiple lines within a cell via
<CR> etc. Also it allows tabs to work. I see that if I put a cell in edit state the TABS do work however, the multiple lines do not.
 
 I feel the block copy and paste is related to the above issues in normal
edit mode that could be getting in the way.

 The only way I found that I can disable this is to intercept the keystrokes
within the KeyUp and Keydown events and zero the KEY value. But that also kills
the standard editing paste mode. I need to check for cell being in edit mode then.
 
 Is it possible to simply have a property to enable/disable the block Copy/Paste and select the delimiter characters for Horizontal and vertical?
Then the Block function would make sense when needed.

K155LA3

2016-09-06 18:03

reporter   ~0094438

> The only way I found that I can disable this is to intercept the keystrokes
within the KeyUp and Keydown events and zero the KEY value. But that also kills
the standard editing paste mode. I need to check for cell being in edit mode then.

You can use next trick. In you module with string grid on form place this code:

-------------
type

...

TStringGrid = class(Grids.TStringGrid)
protected
  procedure DoCopyToClipboard; override;
  procedure DoCutToClipboard; override;
  procedure DoPasteFromClipboard; override;
end;

...

implementation

...

procedure TStringGrid.DoCopyToClipboard;
begin
end;

procedure TStringGrid.DoCutToClipboard;
begin
end;

procedure TStringGrid.DoPasteFromClipboard;
begin
end;
-------------

You can use normaly string grid in design mode, but copy/pase functional will be disabled. I'm use this trick to add undo/redo functional for my program with only Lazarus standart component.

wp

2016-09-06 19:31

developer   ~0094442

@jamie: Regarding cells with linebreaks and tabs, see my patch in the other report (http://bugs.freepascal.org/view.php?id=30556).

Issue History

Date Modified Username Field Change
2016-08-09 21:48 K155LA3 New Issue
2016-08-09 21:48 K155LA3 File Added: TSGBug.gif
2016-08-11 21:14 K155LA3 Note Added: 0094100
2016-08-25 21:24 Jesus Reyes Assigned To => Jesus Reyes
2016-08-25 21:24 Jesus Reyes Status new => assigned
2016-09-01 19:40 Jesus Reyes Fixed in Revision => 52904
2016-09-01 19:40 Jesus Reyes LazTarget => 1.8
2016-09-01 19:40 Jesus Reyes Note Added: 0094374
2016-09-01 19:40 Jesus Reyes Status assigned => resolved
2016-09-01 19:40 Jesus Reyes Fixed in Version => 1.7 (SVN)
2016-09-01 19:40 Jesus Reyes Resolution open => fixed
2016-09-01 19:40 Jesus Reyes Target Version => 1.8
2016-09-03 11:49 K155LA3 Note Added: 0094385
2016-09-03 11:50 K155LA3 Note Edited: 0094385 View Revisions
2016-09-04 09:06 K155LA3 Note Edited: 0094385 View Revisions
2016-09-04 13:10 Bart Broersma Relationship added related to 0030556
2016-09-04 22:14 jamie philbrook Note Added: 0094411
2016-09-05 19:28 Jesus Reyes Note Added: 0094428
2016-09-05 19:29 Jesus Reyes Note Edited: 0094428 View Revisions
2016-09-06 00:21 jamie philbrook Note Added: 0094430
2016-09-06 18:03 K155LA3 Note Added: 0094438
2016-09-06 19:31 wp Note Added: 0094442
2016-12-18 20:19 Jesus Reyes Relationship added related to 0030608
2016-12-18 20:19 Jesus Reyes Relationship added related to 0030607
2016-12-18 20:21 Jesus Reyes Relationship added related to 0030623