Hi,

Would introducing a overload of TStrings.SaveTo* with a 3rd parameter to controle the writing of the BOM be acceptable?

Reason: convenience:
Fpc 3.2 introduces 2 new behaviours of TStrings: Encoding and WriteBom. For those of us who are used to have UTF8 encode textfile without BOM (and want to keep it that way for whatever reason) we have to incorporate 2 changes in our code:
- change calls to TSrings.LoadFrom*/SaveTo* to handle encoding issues
- change WriteBom to False before SaveTo*

SaveToStream(Fn, TEncoding.UTF8, False) is a one line change. Whereas:
WriteBom := False;
SaveToStream(Fn, TEncoding.UTF8) is a two line change.

SaveTo*(Filename, TEncoding) would then call
SaveTo*(Filename, TEncoding, FWriteBom)

I know current method signatures are the same as Delphi, but nothing stops us from being better that the Greek now, do they?

By all means, patches welcome.

Michael.

PM Michael Van Canneyt wrote

By all means, patches welcome.

I can provide a patch for that.
If the patch were to be accepted, would it go into 3.2.0? (Otherwise, see my argument for implementing it, it's hardly worth bothering.)

PS. Happy New Year to all of you.

PM Bart wrote

> By all means, patches welcome.

Patch attached.
For completeness I could implement also SaveToStream(AStream, AWriteBom) and SaveToFile(FileName, AWriteBom).

B.t.w. Am I correct in understanding that when TStrinsg.SaveTo*() without an encoding parameter (so, the 1 parameter version) will NOT write the BOM if Encoding = nil, whereas the SaveTo* with Encoding parameter will write the BOM if Encoding is nil?
(Assuming that WriteBom=True and DefaultEncoding.GetPreamble returns something with Length > 0)

It feels a bit inconsistent to me.
I would have expected that SaveToStream(AStream) would simply call SaveToStream(AStream. Encoding).
It could of course be that Deplhi has implemented it that way (and we must follow), but since my last Delphi version is D7, I cannot test that.

Isn't the TStrings.WriteBOM property good enough? Why to have yet another not-very-useful overload?

Ondrej

// code begin //
-Procedure TStrings.SaveToStream(Stream: TStream; AEncoding: TEncoding);
-
  Var B : TBytes;
  begin
    if AEncoding=nil then
      AEncoding:=FDefaultEncoding;
*-  if FWriteBOM then**
**+  if AWriteBOM then**
*     begin
        B:=AEncoding.GetPreamble;
        if Length(B)>0 then
// code end //

You replace the WriteBOM property with the AWriteBOM parameter. What's the point of it? What do you plan to do with the WriteBOM property then?

Ondrej

PM Ondrej Pokorny wrote

You replace the WriteBOM property with the AWriteBOM parameter. What's the point of it? What do you plan to do with the WriteBOM property then?

I quote myself (from my fiirst post):

SaveToStream(Fn, TEncoding.UTF8, False) is a one line change. Whereas: WriteBom := False; SaveToStream(Fn, TEncoding.UTF8) is a two line change.

It's just for convenience.
No, it is not a necessity.

Since the option to write the BOM or not is only used at write-time, IMHO it would have made more sense to not introduce the WriteBom property, but to have it as an optional parameter for the SaveTo* methods.

I won't loose any sleep over it if it wont make it into fpc. For the record: I did ask before trying...

Ondrej Pokorny wrote

Isn't the TStrings.WriteBOM property good enough? Why to have yet another not-very-useful overload?

To be honest I think Bart's idea of making the BOM an optional parameter of the Save methods appears to me more efficient than defining a property just for this only purpose. Do we really have to imitate all the nonsense dictated by Delphi?

Just make your own descendent - jeez.

AM Werner Pamler wrote

To be honest I think Bart's idea of making the BOM an optional parameter of the Save methods appears to me more efficient than defining a property just for this only purpose. Do we really have to imitate all the nonsense dictated by Delphi?

Well, probably yes.
I can see Ondrej's point though.
SkipLastLineBreak could also have been implemented as another param to SaveTo*. And in the future some new other property/param?

I'll just let the devels decide.

I see it's quite popular to humble every new Delphi feature after Delphi 7 as being nonsense. Not every time the criticism is valid. And never it is a good argument.

The WriteBOM property has a valid purpose - it is set in LoadFrom* according to the BOM presence in the loaded file. I.e. if you do:

MyStrings.LoadFromFile('abc'); // WriteBOM and Encoding properties are
set here
MyStrings.SaveToFile('abc'); // they are used here to save the file as
it was loaded

The abc file keeps BOM and encoding of the original one. You cannot achieve it with the AWriteBOM parameter Bart suggested.

If you want to simplify a call, use a class helper:

TStringsHelper = class helper for TStrings
...
procedure TStringsHelper.SaveToFileWithBOM(aFileName: string);
begin
   WriteBOM := True;
   SaveToFile(aFileName);
end;

Ondrej

I stand corrected. Neither FPC nor Lazarus do it. Obviously my memory doesn't serve me well and I didn't check it before sending the email. Only the encoding is set, not the WriteBOM property.

But still adding an overload just to omit one line for changing some property is nonsense. You can do so in a class helper if you want.

Ondrej

Maybe I am blind here, but our TStrings.LoadFrom* methods do NOT seem to modify the WriteBOM property - otherwise I would not have posted. Why would it be called *Write*BOM if it were also for reading?

Maybe I am blind here, but our TStrings.LoadFrom* methods do NOT seem to modify the WriteBOM property - otherwise I would not have posted. Why would it be called *Write*BOM if it were also for reading?

Where is it coming from?
There's nothing about that in WriteBOM nor LoadFromFile http://docwiki.embarcadero.com/Libraries/Rio/en/System.Classes.TStrings.WriteBOM

http://docwiki.embarcadero.com/Libraries/Rio/en/System.Classes.TStrings.LoadFromFile

From Delphi 10.3

WriteBOM is set to true, by default.
LoadFromFile doesn't affect the value in any manner (no matter if BOM is present in the file or not).
(looking at the delphi code, doesn't suggest that WriteBOM is being changed in any manner either)

thanks,
Dmitry

a bit more accurate version:

procedure TStringsHelper.SaveToFileWithBOM(aFileName: string);
var  prevBom : Boolean;
begin
  prevBom := WriteBOM ;
  try
    WriteBOM := True;
    SaveToFile(aFileName);
  finally
    WriteBom := prevBom;
  end;
end;  :

AM Ondrej Pokorny wrote

But still adding an overload just to omit one line for changing some property is nonsense. You can do so in a class helper if you want.

I did not even consider that option.
May indeed be the better choice.

AM Ozz Nixon wrote

Just make your own descendent - jeez.

That is noy going to solve my problem.
An addition like suggested would have made adjusting code inside Lazarus a bit easier.
There are a multitude of TStrings descendants there, I would have to re-base all of those then.

B.t.w. the "- jeez" part of your reply isn't very helpfull here.

On Wed, Jan 1, 2020 at 1:00 PM Bart wrote:

Meanwhile, can anybody who has a recent Delphi version comment on this:

Is this indeed the case in Delphi?
That would mean that if Strings.Encoding is nil and TStrings.DefaultEncoding is TEncoding.UTF8 then SaveToFile(Fn) would give a different result then SaveToFile(Fn, TEncoding.Utf8).

In Delphi it doesn't matter where the encoding comes from. If writeBOM is true and there is a preamble for the encoding (whatever it is), it is used.

Michael.

PM Michael Van Canneyt wrote

In Delphi it doesn't matter where the encoding comes from. If writeBOM is true and there is a preamble for the encoding (whatever it is), it is used.

I think I have to rephrase my question.
In fpc TStrings.SaveToStream(AStream) does NOT use DefaultEncoding if Encoding is nil.
So this code:

//Source codepage is Windows 1512
SL := TStringList.Create;
SL.SkipLastLineBreak := True;
SL.WriteBOM := True;
SL.DefaultEncoding := TEncoding.UTF8;
S := 'ä'; // = #$0E
SL.Add(S);
SL.SaveToStream(St)

Will result in St having as content 1 single byte: 0E (hex)

Whereas
SL.SaveToStream(St, nil) will result in St having a content of 5 bytes: EF BB BF C3 A4

Is that Delphi compatible?

From the help text of the Embarcadero site I cannot figure that out.

That seems not compatible to me.

Michael.

Should be fixed in r43839. Please report any issues.

Ondrej

PM Ondrej Pokorny wrote

Should be fixed in r43839. Please report any issues.

That was quick!
Thanks.