unit Grids.pas SaveToCSVStream bug?
lazarus / 7-LCL

----------------------------------------------------------------------
KodeZwerg · 2022-12-19 05:26

Hello there, I do actually not know if this is on purpose or a bug,
here is what I found out.
I made a basic Grid and entered in the cells (beside fixed cells) test
data.
When I did exported a file, it somehow had strange content.
The SaveToCSVStream mechanism add infront of each line a delimiter
char when you turned WriteTitles off.
here is the original:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i procedure
TCustomStringGrid.SaveToCSVStream(AStream: TStream; ADelimiter: Char; 
WriteTitles: boolean=true; VisibleColumnsOnly: boolean=false);var 
i,j,StartRow: Integer;  HeaderL, Lines: TStringList;  C:
TGridColumn;begin  if (RowCount=0) or (ColCount=0) then    exit; 
Lines := TStringList.Create;  try    if WriteTitles then begin      if
Columns.Enabled then begin        if FixedRows>0 then begin         
HeaderL := TStringList.Create;          try            // Collect
header column names to a temporary StringList            for i := 0 to
ColCount-1 do begin              c := ColumnFromGridColumn(i);       
     if (c <> nil) then begin                if c.Visible or not
VisibleColumnsOnly then                 
HeaderL.Add(c.Title.Caption);              end              else     
       if not VisibleColumnsOnly then               
HeaderL.Add(Cells[i, 0]);            end;           
HeaderL.Delimiter:=ADelimiter;            Headerl.StrictDelimiter :=
False; //force quoting of strings that contain whitespace or
Delimiter            Lines.Add(HeaderL.DelimitedText); // Add as a
first row in Lines          finally            HeaderL.Free;         
end;        end;        StartRow := FixedRows;      end else      if
FixedRows>0 then        StartRow := FixedRows-1      else       
StartRow := 0;    end else      StartRow := FixedRows;    for
i:=StartRow to RowCount-1 do begin      if Columns.Enabled and
VisibleColumnsOnly then begin        HeaderL := TStringList.Create;  
    try        for j := 0 to ColCount-1 do begin          c :=
ColumnFromGridColumn(j);          if c=nil then Continue;          if
c.Visible then            HeaderL.Add(Cells[j,i]);        end;       
HeaderL.Delimiter:=ADelimiter;        HeaderL.StrictDelimiter :=
False; //force quoting of strings that contain whitespace or
Delimiter        Lines.Add(HeaderL.DelimitedText); // Add the row in
Lines        finally          HeaderL.Free;        end;      end     
else      begin      Rows[i].StrictDelimiter := False; //force quoting
of strings that contain whitespace or Delimiter     
Rows[i].Delimiter:=ADelimiter;      Lines.Add(Rows[i].DelimitedText); 
  end;    end;    Lines.SaveToStream(AStream);  finally   
Lines.Free;  end;end;
here is my patch:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i procedure
TCustomStringGrid.SaveToCSVStream(AStream: TStream; ADelimiter: Char; 
WriteTitles: boolean=true; VisibleColumnsOnly: boolean=false);var 
i,j,StartRow: Integer;  HeaderL, Lines: TStringList;  C:
TGridColumn;begin  if (RowCount=0) or (ColCount=0) then    exit; 
Lines := TStringList.Create;  try    if WriteTitles then begin      if
Columns.Enabled then begin        if FixedRows>0 then begin         
HeaderL := TStringList.Create;          try            // Collect
header column names to a temporary StringList            for i := 0 to
ColCount-1 do begin              c := ColumnFromGridColumn(i);       
     if (c <> nil) then begin                if c.Visible or not
VisibleColumnsOnly then                 
HeaderL.Add(c.Title.Caption);              end              else     
       if not VisibleColumnsOnly then               
HeaderL.Add(Cells[i, 0]);            end;           
HeaderL.Delimiter:=ADelimiter;            Headerl.StrictDelimiter :=
False; //force quoting of strings that contain whitespace or
Delimiter            Lines.Add(HeaderL.DelimitedText); // Add as a
first row in Lines          finally            HeaderL.Free;         
end;        end;        StartRow := FixedRows;      end else      if
FixedRows>0 then        StartRow := FixedRows-1      else       
StartRow := 0;    end else      StartRow := FixedRows;    for
i:=StartRow to RowCount-1 do begin      if Columns.Enabled and
VisibleColumnsOnly then begin        HeaderL := TStringList.Create;  
    try        for j := 0 to ColCount-1 do begin          c :=
ColumnFromGridColumn(j);          if c=nil then Continue;          if
c.Visible then            HeaderL.Add(Cells[j,i]);        end;       
HeaderL.Delimiter:=ADelimiter;        HeaderL.StrictDelimiter :=
False; //force quoting of strings that contain whitespace or
Delimiter        Lines.Add(HeaderL.DelimitedText); // Add the row in
Lines        finally          HeaderL.Free;        end;      end     
else      begin        HeaderL := TStringList.Create;        try     
 for j := FixedCols to ColCount-1 do begin           
HeaderL.Add(Cells[j,i]);            HeaderL.Delimiter:=ADelimiter;   
       HeaderL.StrictDelimiter := False; //force quoting of strings
that contain whitespace or Delimiter        end;       
Lines.Add(HeaderL.DelimitedText); // Add the row in Lines       
finally          HeaderL.Free;        end;    end;    end;   
Lines.SaveToStream(AStream);  finally    Lines.Free;  end;end;
If you think its useful feel free to update Grids.pas with my patch.
If you think it is wrong, please enlight me the truth to let me see my
mistake.

In attachment are two images, the first show the unpatched Grids.pas
The second demonstrate the patched varation

My Version: Lazarus 2.2.4 (rev lazarus_2_2_4) FPC 3.2.2
x86_64-win64-win32/win64

----------------------------------------------------------------------
rvk · 2022-12-19 10:09

Quote from: KodeZwerg on December 19, 2022, 06:26:07 amThe
SaveToCSVStream mechanism add infront of each line a delimiter char
when you turned WriteTitles off.
It's not that strange. The first field is the text in the first column
(your fixed column).
Setting WriteTitles to false only eliminates the FixedRow rows. Not
the FixedCol columns.
You could create a WriteFixed to also eliminate the fixed column (or
the WriteTitles should be renamed to WriteHeaders).

Anyway... looking at your code I see a bit of inefficiency (ever so
slightly). You do the Delimiter:=ADelimiter and StrictDelimiter :=
False for EVERY column.
This would be better (this was probably because the original code
isn't really properly indented):
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i HeaderL :=
TStringList.Create;try  for j := FixedCols to ColCount-1 do
HeaderL.Add(Cells[j,i]);  HeaderL.Delimiter:=ADelimiter; 
HeaderL.StrictDelimiter := False; //force quoting of strings that
contain whitespace or Delimiter  Lines.Add(HeaderL.DelimitedText); //
Add the row in Linesfinally  HeaderL.Free;end;
Also... what if I really WANT the FixedCol exported in my CSV? It
could contain really important information.
In that case you should look at WriteTitles and only start from
FixedCols is that is false.

Something like (not tested):
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i if
WriteTitles then StartCol := 0 else StartCol := FixedCols;for j :=
StartCol to ColCount-1 do HeaderL.Add(Cells[j,i]);

----------------------------------------------------------------------
wp · 2022-12-19 10:35

The author of the SaveToCSVFile (not me) decided to mean with "Titles"
only the headers of the columns because a stringgrid with GridColumns
has "Titles" in the top-most row. If a grid contains no gridcolumns
the "Titles" are the cells in the fixed rows. The cells in the fixed
columns are not understood to be "Titles" in this concept. This is by
design, not a bug.

The fixed column containing the row headers thus is not affected by
the "WriteTitles" parameter. So what appears to be a leading semicolon
to you is just the consequence of the the first column being empty.

Since your fixed column is empty, simply turn it off (Grid.FixedCols
:= 0), and everything will be fine.

Of course, if you have content in the fixed columns but you do not
want to save it you have a problem. In this case I'd recommend to use
an own procedure like yours.

It would be possible to extend the procedure by an additional
parameter, "WriteFixedCols", to clarify things. This would not be very
nice because the original parameters (WriteTitles, VisibleColumnsOnly)
must be kept in place for compatibility reasons and therefore
WriteTitles and WriteFixedCols would not be listed together which I
personally would expect. One could overload the entire procedure and
introduce some "CSVOptions = set of (coWriteTitles, coWriteFixedCols,
csVisibleColumnsOnly)", but sincerely, I don't like this either
because I think that saving to CSV should not be a method of the grids
at all. What if somebody wants to save a grid to hmtl, xml, json, xls?
For this the user would have to write his own procedures anyway. So
why not drop the SaveToCSVFile at all and introduce a more general
TGridExporter?

----------------------------------------------------------------------
KodeZwerg · 2022-12-19 10:45

Thanks @rvk for hint with removing the wrong begin/end, yeah I just
copied and pasted without seeing it  :'(
My own method does do same and on wish also like it was before i
accidently felt over it 
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i procedure
SaveGridToFile(const AStringGrid: TStringGrid; const AFileName:
string; const ADelimiter: Char = ','; const AIncludeFixed: Boolean =
False; const AIncludeTitle: Boolean = False);var  LRow, LCol, 
LRowStart, LColStart: Integer;  sl: TStringList;  s: string;begin  sl
:= TStringList.Create;  if AIncludeFixed then    begin      LRowStart
:= 0;      LColStart := 0;    end    else    begin      if
AIncludeTitle then        LRowStart := 0        else        LRowStart
:= AStringGrid.FixedRows;      LColStart := AStringGrid.FixedCols;   
end;  try    for LRow := LRowStart to Pred(AStringGrid.RowCount) do  
  begin        s := '';        for LCol := LColStart to
Pred(AStringGrid.ColCount) do          if (LCol <
Pred(AStringGrid.ColCount)) then            s := s +
AStringGrid.Cells[LCol, LRow] + ADelimiter            else           
s := s + AStringGrid.Cells[LCol, LRow];        sl.Add(s);      end;  
sl.SaveToFile(AFilename, True);  finally    sl.Free;  end;end;

----------------------------------------------------------------------
rvk · 2022-12-19 10:55

Quote from: KodeZwerg on December 19, 2022, 11:45:04 amCode: Pascal
 [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i s := s +
AStringGrid.Cells[LCol, LRow] + ADelimiterYou definitely want to fix
these lines 
Using a TStringList.DelimitedText, there is some logic which adds
protection for a delimiter character in a string.
In your code you don't.

So if one of you cells contain a ADelimiter character, the whole CSV-
line would be corrupt.

----------------------------------------------------------------------
KodeZwerg · 2022-12-19 11:07

Quote from: wp on December 19, 2022, 11:35:08 amSince your fixed
column is empty, simply turn it off (Grid.FixedCols := 0), and
everything will be fine.When I do that, it be same like on my first
post screensnap, leading delimiter 
Thanks for all of your text, I understand now that this is a wanted
behavior and my thinking was wrong.

----------------------------------------------------------------------
rvk · 2022-12-19 11:13

Quote from: KodeZwerg on December 19, 2022, 12:07:31 pmQuote from: wp
on December 19, 2022, 11:35:08 amSince your fixed column is empty,
simply turn it off (Grid.FixedCols := 0), and everything will be
fine.When I do that, it be same like on my first post screensnap,
leading delimiter 
After doing that (setting FixedCols to 0) you would need to reload the
text in your grid, and your first real field will end up in column 0.
In that case you don't have a first empty column (but you will also
loose a visible margin column).

----------------------------------------------------------------------
wp · 2022-12-19 11:31

Quote from: KodeZwerg on December 19, 2022, 12:07:31 pmmy thinking was
wrong.
Hmmm... I think there's no wrong or right here, it's just a matter of
preferences. The problem is that once a procedure was written and has
been released it is hard to change it without break existing code.

----------------------------------------------------------------------
wp · 2022-12-19 11:37

Quote from: KodeZwerg on December 19, 2022, 12:07:31 pmQuote from: wp
on December 19, 2022, 11:35:08 amSince your fixed column is empty,
simply turn it off (Grid.FixedCols := 0), and everything will be
fine.When I do that, it be same like on my first post screensnap,
leading delimiter 
Sorry. Setting Grid.FixedCells to 0 simply converts the cells in the
first column (index 0) from fixed cells to normal cells, and then the
SaveToCSVFile will still be the same because it does not distinguish
between normal and fixed cells.

What I meant is: Set Grid.FixedCols=0 before you fill the grid and
when you fill it make sure that every column index is decremented by 1
compared with the current code.

----------------------------------------------------------------------
KodeZwerg · 2022-12-19 11:44

Quote from: rvk on December 19, 2022, 11:55:25 amQuote from: KodeZwerg
on December 19, 2022, 11:45:04 amCode: Pascal
 [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i s := s +
AStringGrid.Cells[LCol, LRow] + ADelimiterYou definitely want to fix
these lines 
Using a TStringList.DelimitedText, there is some logic which adds
protection for a delimiter character in a string.
In your code you don't.

So if one of you cells contain a ADelimiter character, the whole CSV-
line would be corrupt.

I agree, I was not that deep thinking about such situation and fixed
it.
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i //
SaveGridToFile method by KodeZwerg for FreePascal forum// this method
can plain save grid content into a text file where each element is
seperated by a delimiter (default = ",")// argument #1: the stringgrid
that you want to save// argument #2: the filepath and filename to
save, no checking if valid// argument #3: optional set a delimiter,
default ","// argument #4: optional shall the save include fixed
cells, default False// argument #5: optional shall the save include
the title cells, default Falseprocedure SaveGridToFile(const
AStringGrid: TStringGrid; const AFileName: string; const ADelimiter:
Char = ','; const AIncludeFixed: Boolean = False; const AIncludeTitle:
Boolean = False);var  LRow, LCol, // where are we inside loop 
LRowStart, LColStart: Integer; // where do we start working at  sl,
hl: TStringList; // sl is the main list with formated results from
hlbegin  sl := TStringList.Create; // create object, needed in the
main loop  try    hl := TStringList.Create; // create object, needed
in the main loop to create the sl list    try      hl.Delimiter :=
ADelimiter; // setup Delimiter      hl.StrictDelimiter := False;
//force quoting of strings that contain whitespace or Delimiter     
if AIncludeFixed then // user wants to include the fixed cells in the
results        begin          if AIncludeTitle then // user wants to
include the title            LRowStart := 0 // giving every row      
    else            if ((AStringGrid.FixedRows >= 1) and
(AStringGrid.RowCount > 0)) then // giving fixed row without title   
         LRowStart := 1 // we cut the title off              else    
        LRowStart := 0; // giving all          LColStart := 0; // 0
for the fixed beginning        end // user wants to include the fixed
cells in the results        else        begin // user does not want to
have fixed cells          if AIncludeTitle then // user wants to
include title (what results in getting all cells)            LRowStart
:= 0 // giving all            else            LRowStart :=
AStringGrid.FixedRows; // user want no fixed row data         
LColStart := AStringGrid.FixedCols; // user want no fixed col data   
   end; // user does not want to have fixed cells      for LRow :=
LRowStart to Pred(AStringGrid.RowCount) do // begin outer loop for the
rows        begin          hl.Clear;  // clear stringlist          for
LCol := LColStart to Pred(AStringGrid.ColCount) do // begin inner loop
for the columns            hl.Add(AStringGrid.Cells[LCol, LRow]); //
transport string-grid-cell to stringlist         
sl.Add(hl.DelimitedText); // store the generated delimited       
end;    finally      hl.Free; // free the hl list    end;    if
(sl.Count > 0) then // we gathered something, lets save     
sl.SaveToFile(AFilename, True); // save main list to disk            
                        // this method assume that you want to save
and overwrite                                      // so check outside
of this method your filename on disk etc...  finally    sl.Free; //
free the main list  end;end;