View Issue Details

IDProjectCategoryView StatusLast Update
0036249FPCRTLpublic2019-11-06 11:55
Reporterjamie philbrookAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.0.4Product Build 
Target VersionFixed in Version3.3.1 
Summary0036249: Feature for TStringList.AddCommaText
DescriptionSince we are adding things to TstringList, it would be nice if we could have a AddCommaText Procedure or write only property.

Currently If I want to add a CommaText string to a list I have to create an stringlist, populate that one with the commaText and then add it to the base stringlist.

 The idea is to quickly populate a list with condensed lines and at some point you need to break and later come back to add more commatext to it without losing what you already have.

 This could be accomplished in a neat manner if we did it via the class..

I think I can code this with a Helper if you want to see examples, or do a local class hack to demonstrate it.
TagsNo tags attached.
Fixed in Revision43405
FPCOldBugId
FPCTarget3.2.0
Attached Files
  • project1.zip (128,450 bytes)
  • project1-2.zip (128,714 bytes)
  • classes_adddelimitedtext.patch (1,783 bytes)
    Index: rtl/objpas/classes/classesh.inc
    ===================================================================
    --- rtl/objpas/classes/classesh.inc	(revision 43385)
    +++ rtl/objpas/classes/classesh.inc	(working copy)
    @@ -696,6 +696,9 @@
         procedure AddStrings(const TheStrings: array of string); overload; virtual;
         procedure AddStrings(const TheStrings: array of string; ClearFirst : Boolean); overload;
         Procedure AddText(Const S : String); virtual;
    +    procedure AddCommaText(const S: String);
    +    procedure AddDelimitedText(const S: String; ADelimiter: char; AStrictDelimiter: Boolean); overload;
    +    procedure AddDelimitedtext(const S: String); overload;
         procedure Append(const S: string);
         procedure Assign(Source: TPersistent); override;
         procedure BeginUpdate;
    Index: rtl/objpas/classes/stringl.inc
    ===================================================================
    --- rtl/objpas/classes/stringl.inc	(revision 43385)
    +++ rtl/objpas/classes/stringl.inc	(working copy)
    @@ -926,6 +926,40 @@
       DoSetTextStr(S,False);
     end;
     
    +procedure TStrings.AddCommaText(const S: String);
    +var
    +  L: TStringList;
    +begin
    +  L := TStringList.Create;
    +  try
    +    L.CommaText := S;
    +    AddStrings(L);
    +  finally
    +    L.Free;
    +  end;
    +end;
    +
    +procedure TStrings.AddDelimitedText(const S: String; ADelimiter: Char;
    +  AStrictDelimiter: Boolean);
    +var
    +  L: TStringList;
    +begin
    +  L := TStringList.Create;
    +  try
    +    L.Delimiter := ADelimiter;
    +    L.StrictDelimiter := AStrictDelimiter;
    +    L.DelimitedText := S;
    +    AddStrings(L);
    +  finally
    +    L.Free;
    +  end;
    +end;
    +
    +procedure TStrings.AddDelimitedText(const S: String);
    +begin
    +  AddDelimitedText(S, FDelimiter, FStrictDelimiter);
    +end;
    +
     Procedure TStrings.SetUpdateState(Updating: Boolean);
     
     begin
    

Activities

jamie philbrook

2019-11-02 20:37

reporter  

project1.zip (128,450 bytes)

jamie philbrook

2019-11-02 21:11

reporter   ~0118991

I wanted to add that I forgot to transpose over the Delimiter and Strictdelimiter properties so currently it uses the default but this is just a
concept model.. I would expect it to be integrated into the class more efficiently if the concept is accepted.

wp

2019-11-02 22:17

reporter   ~0118994

If such a method is added then it should be a more general TStrings.AddDelimitedText(AText: String; ADelimiter:Char; AStrictDelimiter: boolean).

jamie philbrook

2019-11-02 22:32

reporter   ~0118996

Sure that sounds very good, that way it would be used for starting empty list or adding them to existing list..
and maybe set the AStrictDelimiter:Boolean = False to sweeten the deal ;)

 I suppose you could even make the ADelmmiter:Char = ',' as a default too...

 So it could be called with three levels deep.
 (String)
 (String, Char)
 (String, Char, Boolean)

 That's good thinking @WP ;)

jamie philbrook

2019-11-02 23:11

reporter   ~0118997

Last edited: 2019-11-02 23:51

View 3 revisions

I could not figure out how to get past the compiler issue of it not being able to determine which one to call because I was trying to make
overloaded procedures so that it could be called in steps but this is what I came up with that works with the 3.0.4 and assume it will work with the newer ones.
  
 What this does is uses a #0 for the delimiter char and ByteBool($80) for the strict property.
This way the code can decide to use the values in the base stringlist instead and it shortens the source code calls, does not shorten actual code in the background but makes is easier for the coder.
Tested and it works with all three combinations

procedure TstringListHelper.AddDelimiterText(AString: string;
  ADelimiterChar: char = #0; AStrictDelimiter: bytebool = bytebool($80));
var
  S: TStringList;
begin
  S := TStringList.Create;
  if ADelimiterChar = #0 then
    S.Delimiter := Self.Delimiter
  else
    S.Delimiter := ADelimiterChar;
  if byte(AStrictDelimiter) = $80 then
    S.StrictDelimiter := Self.StrictDelimiter
  else
    S.Strictdelimiter := AStrictDelimiter;
  S.DelimitedText := Astring; { this was corrected from CommaText }
  Self.AddStrings(S);
  S.Free;
end;

jamie philbrook

2019-11-03 03:21

reporter   ~0118999

I finalized the code, I had to put it back close to how the TstringList works already because things were getting out of control.

So we have this
Property AddCommaText:String;
Property AddDelimitedText:String;
Procedure CustomAddDelimitedText(AString; AdelimiterChar; AStrictDelimiter:Boolean);

-- Attached is the latest project file that contains the helper Class --
I hope this can get accepted as an add in for the Class because otherwise its a pain doing this the old way.

project1-2.zip (128,714 bytes)

wp

2019-11-03 22:29

reporter   ~0119026

> otherwise its a pain doing this the old way.

What is the "pain" in this three-liner?

procedure AddDelimited(AList: TStrings; ANew: String; ADelimiter: Char);
var
  sa: TStringArray;
  s: String;
begin
  sa := ANew.Split(ADelimiter);
  for s in sa do
    AList.Add(s);
end;

jamie philbrook

2019-11-03 23:01

reporter   ~0119029

Did you look at the second project-2.zip I attached ?

Anyways, I understand the concerned of adding functions to the rtl that can be done outside within the users code however, there is a lot that can be said about what has already been done weighing down app space with dead code.

  I like to add value when the value of the type that can get used is on a large scale. Delimiter text functions are very common for loading list and parsing strings and so on.

 I suppose years ago Delphi took it off the wrong foot, they should of called it "Add..." instead of just replacing the whole boady, that way one could simply use the CLEAR prior to add.... but that is not how it happened and here we are today looking to make the tools better so the user's code is cleaner looking.

  Now I suppose if the team wanted to get brave they could modify the stringlist with a parameter to indicate if those existing functions should add or replace the body and leave them as they are now, but still requires some code writing.

 Oh well, cancel this idea if you wish, I'll just make units and include them in all of my projects..

wp

2019-11-04 10:04

reporter   ~0119039

Last edited: 2019-11-04 10:15

View 2 revisions

Yes, I am not a friend of adding more and more rarely used features which could easily be implemented by the user when he needs them. For example, the overloaded method "AddStrings" with additional parameter "ClearFirst" which saves the user from calling "Clear" before the conventional "AddStrings" is not necessary to me.

I understood your sentence "otherwise its a pain doing this the old way" as if implementing an "AddDelimitedText" would not be easy, and this motivated me to my answer - but maybe I misunderstood you.

Anyway, there are already a lot of Add* methods in the TStrings class, and having an AddDelimitedText and an AddCommaText would complete them, so, I do support your feature request in this case. If it gets added I hope that it will be added to TStrings, not to TStringList (as suggested in the title), because otherwise a call such as "Listbox.Items.AddDelimitedText('A;B;C')" would not be possible.

BTW, I do not like to have it as a property because this is not in line with the other Add* features (AddText, AddStrings, AddPair) which are available as methods.

wp

2019-11-04 11:19

reporter   ~0119043

I am adding a patch for TStrings.

classes_adddelimitedtext.patch (1,783 bytes)
Index: rtl/objpas/classes/classesh.inc
===================================================================
--- rtl/objpas/classes/classesh.inc	(revision 43385)
+++ rtl/objpas/classes/classesh.inc	(working copy)
@@ -696,6 +696,9 @@
     procedure AddStrings(const TheStrings: array of string); overload; virtual;
     procedure AddStrings(const TheStrings: array of string; ClearFirst : Boolean); overload;
     Procedure AddText(Const S : String); virtual;
+    procedure AddCommaText(const S: String);
+    procedure AddDelimitedText(const S: String; ADelimiter: char; AStrictDelimiter: Boolean); overload;
+    procedure AddDelimitedtext(const S: String); overload;
     procedure Append(const S: string);
     procedure Assign(Source: TPersistent); override;
     procedure BeginUpdate;
Index: rtl/objpas/classes/stringl.inc
===================================================================
--- rtl/objpas/classes/stringl.inc	(revision 43385)
+++ rtl/objpas/classes/stringl.inc	(working copy)
@@ -926,6 +926,40 @@
   DoSetTextStr(S,False);
 end;
 
+procedure TStrings.AddCommaText(const S: String);
+var
+  L: TStringList;
+begin
+  L := TStringList.Create;
+  try
+    L.CommaText := S;
+    AddStrings(L);
+  finally
+    L.Free;
+  end;
+end;
+
+procedure TStrings.AddDelimitedText(const S: String; ADelimiter: Char;
+  AStrictDelimiter: Boolean);
+var
+  L: TStringList;
+begin
+  L := TStringList.Create;
+  try
+    L.Delimiter := ADelimiter;
+    L.StrictDelimiter := AStrictDelimiter;
+    L.DelimitedText := S;
+    AddStrings(L);
+  finally
+    L.Free;
+  end;
+end;
+
+procedure TStrings.AddDelimitedText(const S: String);
+begin
+  AddDelimitedText(S, FDelimiter, FStrictDelimiter);
+end;
+
 Procedure TStrings.SetUpdateState(Updating: Boolean);
 
 begin

jamie philbrook

2019-11-04 23:20

reporter   ~0119062

Thanks, that looks perfect. I'll adjust my helper I am using and code base to adapt to using what you have so that it will drop in naturally when this code gets out there.

 Thanks again.

Michael Van Canneyt

2019-11-06 11:54

administrator   ~0119096

Normally I would not add this.

The functional methods I added because they add more generic "functional" mechanisms, often seen in other languages such as Javascript, They can be used for anything.

As Werner points out, the 2 added methods are so specific that they can be implemented in a helper which can be used when needed.

But seeing that Werner already created the patch with an implementation as I would implement it, I applied it.
Thanks !


Issue History

Date Modified Username Field Change
2019-11-02 19:51 jamie philbrook New Issue
2019-11-02 20:37 jamie philbrook File Added: project1.zip
2019-11-02 21:11 jamie philbrook Note Added: 0118991
2019-11-02 22:17 wp Note Added: 0118994
2019-11-02 22:32 jamie philbrook Note Added: 0118996
2019-11-02 23:11 jamie philbrook Note Added: 0118997
2019-11-02 23:12 jamie philbrook Note Edited: 0118997 View Revisions
2019-11-02 23:51 jamie philbrook Note Edited: 0118997 View Revisions
2019-11-03 03:21 jamie philbrook File Added: project1-2.zip
2019-11-03 03:21 jamie philbrook Note Added: 0118999
2019-11-03 22:29 wp Note Added: 0119026
2019-11-03 23:01 jamie philbrook Note Added: 0119029
2019-11-04 10:04 wp Note Added: 0119039
2019-11-04 10:15 wp Note Edited: 0119039 View Revisions
2019-11-04 11:19 wp File Added: classes_adddelimitedtext.patch
2019-11-04 11:19 wp Note Added: 0119043
2019-11-04 23:20 jamie philbrook Note Added: 0119062
2019-11-06 11:54 Michael Van Canneyt Assigned To => Michael Van Canneyt
2019-11-06 11:54 Michael Van Canneyt Status new => resolved
2019-11-06 11:54 Michael Van Canneyt Resolution open => fixed
2019-11-06 11:54 Michael Van Canneyt Fixed in Revision => 43405
2019-11-06 11:54 Michael Van Canneyt FPCTarget => -
2019-11-06 11:54 Michael Van Canneyt Note Added: 0119096
2019-11-06 11:55 Michael Van Canneyt Fixed in Version => 3.3.1
2019-11-06 11:55 Michael Van Canneyt FPCTarget - => 3.2.0