Taazz nice work on SDFDataset - and my nasty test code lazarus / 16-Databases ---------------------------------------------------------------------- BigChimp · 2012-09-15 12:51 Taazz & everybody, I've been stripping out your comments and FPC 2.6 adaptations on your sdfdata.pp (in http://lazarus.freepascal.org/index.php/topic,18214.0.html) so that it can be prepared as a patch for current FPC trunk. BTW: may I say I hate Delphi's SDF "format" with a passion? It's so close to CSV yet it's not CSV. Delphi's own documentation is in contradiction at least with what Turbo Delphi 2006 produces. For my synopsis of what sdf should do, please see http://wiki.lazarus.freepascal.org/CSV#SDF_format Michael Van Canneyt graciously commited a test program of mine in FPC trunk: packages/fcl-db/tests/tcsdfdata.pp Not everybody is working with trunk and the test program has some flaws too (e.g. field length). I've been running a modified test program with taazz' code to get it to spit out 0 errors. Unfortunately, it doesn't yet, completely. TestInputOurFormat seems to show that taazz' adaptations work: sdfdataset now correctly reads multiline data!!  I'm having 2 problems: 1. The TestOutput test demonstrates sdfdataset thinks the last record is duplicate. Is it my test or is it the sdfdataset code? The same symptom occurs when reading sdfdataset from file, see the TestDelimitedTextOutput test 2. It still does not deal with quotes (or lack of it) well: this single field (yes, the data includes the quotes) Quote"Delimiter,""and"";quote" gets read and split up into multiple fields. As far as I understand that shouldn't happen, right? Attached taazz' sdfdata.pp adapted for FPC trunk (please rename sdfdata.pp.2.6 if running 2.6.0) and the test program files. Please put them in the same directory, compile them, and run with Code: [Select]testmultiline --all --format=plain Comments, advice, patches gratefully received! ---------------------------------------------------------------------- BigChimp · 2012-09-15 13:52 Sorry, stupid error in testsdfmultiline. fixed version attached. Time for a break, I think... ---------------------------------------------------------------------- BigChimp · 2012-09-15 14:29 And now the debugger on Laz trunk doesn't want to stop on breakpoints. Stabs, dwarf, auto doesn't matter. Where did that last strand of my hair go? ---------------------------------------------------------------------- taazz · 2012-09-15 14:51 well I can give you some of mine their laying all around me if that would help. I'm going for a re installation my self as well. that is why I hate building the IDE to install components 1 wrong move or switch in one of those packages and I'm left in the cold trying to find which of the thousands of switches does not work any more. Oh well an other hour down the drain I suppose. ---------------------------------------------------------------------- BigChimp · 2012-09-15 17:12 Quote from: BigChimp on September 15, 2012, 04:29:16 pmAnd now the debugger on Laz trunk doesn't want to stop on breakpoints. Stabs, dwarf, auto doesn't matter. Where did that last strand of my hair go? Thanks to ludob for figuring it out: passing --all in run parameters doesn't work; passing -a or "--all" does work reported as http://bugs.freepascal.org/view.php?id=22893 ---------------------------------------------------------------------- BigChimp · 2012-09-15 18:05 ... and something more: loading an empty file with FirstLineAsSchema results in a recordcount of 1. That's probably also the case for all other record counts... http://bugs.freepascal.org/view.php?id=22894 I can't believe nobody wrote a test harness for this piece of #()%#()$*#$%#@ before. ---------------------------------------------------------------------- taazz · 2012-09-15 18:22 just a heads up, I'm going to look in to this as well although it might need some heavy restructuring, the way it currently is structured gives me the impression that some one placed code where ever it was convenient not where it supposed to go. For example I am steel looking where is the field separation code and how it works. I found something that looks like it in the InternalInitFieldDefs which has nothing to do with fields position in the buffer but I didn't had time to verify it yet, it might be parsing of the schema line. ---------------------------------------------------------------------- BigChimp · 2012-09-15 18:25 Ok. I'm going to lay off it for a while. If you are going to edit & if you want to, no need to add the comments etc (except those helpful ones for FPC 2.6.0 compatibility that indicate what I need to strip for FPC trunk). I can use winmerge or diff to figure out the differences to the current version. Edit: if you would be so kind though, perhaps we could write up as many test cases as possible... would help keeping track of regressions etc). (OT: And use my own local mercurial repository to keep track of what I do - others swear by subversion or git) Anyway, thanks a lot for the great help up to now Thanks, BigChimp ---------------------------------------------------------------------- BigChimp · 2012-09-16 02:02 Latest test code: - contains taazz code for fpc 2.6 as well as trunk - incorporates expanded test data set (includes expanded tcsdfdata.pp). - Keeps existing console test runner (testmultiline) Run with testmultiline -a --format=plain - Added GUI test runner: testgui.lpi. Runs the same tests but easier to see results within Lazarus or outside ---------------------------------------------------------------------- BigChimp · 2012-09-17 05:16 I agree that sdfdataset probably could do with a rewrite. Things that worry me are e.g. the short fixed field length you seem to get when you have a header line in the file, and the fact that, as you said, taazz, quoting support etc had to be bolted on making it very unwieldy. Meanwhile, extended test coverage is in the dbtest framework in fpc trunk. (make sure to copy database.ini.txt to database.ini and modify the chosen connector from bufdataset to sdfdataset). You can then run e.g. the gui test runner project in the db tests directory. Perhaps we could use bufdataset to store the data which would only leave reading/parsing and saving the sdf format from/to file. We would inherit bufdataset's bugs etc but as SQL database support is based on this, there is an incentive to fix these (and perhaps the bugs won't affect us) I think I'm going to trial this with a new csvdataset based on csvdocument (see wiki) to parse csv/tab separated values/x-separated values. If that works, I'll have a look at sdfdataset. ---------------------------------------------------------------------- taazz · 2012-09-17 13:14 Ok. Downloaded your files. I have a working development folder, thanks to fpcup, so, I'll start running some test cases to see what is going on and how can I improve it tonight. I'll post any findings here. ---------------------------------------------------------------------- taazz · 2012-09-19 04:26 find attached the new and improved TSDFDataset and its test cases. The changes are not as extensive as I was afraid. So far your tests run correctly. I had to change one or two tests because they didn't quote properly other than that I haven' t touched anything else. The current changes are 1) RecordCount does not include the schema line any more. 2) FieldDefs can be used to define a schema instead of the schema list. In the case that both schema and fieldDefs are set then Schema takes presents. 3) some minor changes and a possible infinite loop problem that I need to test. 4) set the foundations for a more validation strict dataset. I need to map how to handle differences in the file schema and fieldDefs collection first. PS. FPC 2.6.1 does not need the sdfdata26.pp so I haven't touched that one. ---------------------------------------------------------------------- BigChimp · 2012-09-19 11:10 Thanks a lot, taazz! I don't really see why you change the delim.csv test to include quoting  given this remark in the code: Quote// See test results from bug 19610 for evidence that the strings below should work.In other words: Delphi emits these kinds of strings as valid sdfdata. It should make sense sdfdataset can then read them? I'll double check the output; using a stringlist and relying on FPC delimited text was not a smart thing to do as bugs in FPC delimitedtext will lead to test failure here. Better to just write the raw string to file... I'll adapt the test Edit: the test does need to be adapted... please wait Could you/anybody confirm that Delphi generates Quote"Delimiter,""and"";quote","J""T""",Just a long line,"Just a quoted long line","multi line","Delimiter,and;done","Some ""random"" ""quotes"if so... I'll add it to the test set as another test. Thanks again, BigChimp ---------------------------------------------------------------------- taazz · 2012-09-19 12:13 Quote from: BigChimp on September 19, 2012, 01:10:53 pmThanks a lot, taazz! I don't really see why you change the delim.csv test to include quoting  given this remark in the code: Quote// See test results from bug 19610 for evidence that the strings below should work. Two reasons 1) I was focused on the CSV RFC (I think is included in the zip I attached) which clearly states that a quote char inside a field is illegal unless the field is quoted. 2) I posted the changes which I think make the component easier to use and tries to merge FieldDefs and schema. There are a number of issues I'm thinking to address on top of the existing base when I get more time, for example Use FieldDefs as a validation mechanism to ensure that the file opened is in the format the user expects ee raise an error if the field number is not enough for all the fielddefs and ignore the extra fields while using the dataset but not when saving the data back to disk.Support field types other than string and the existing validation/editing mechanism that the DB framework has build in.add Support for more specialized types with constrains eg blobs/images which will be saved encoded to either uuencode or enc64.The constrain here that the size will be the maximum length of bytes allowed in each blob and will never save blob larger than the size specifiedchange the way that spaces are handled to avoid trimming spaces that are part of the fields value either typed by the user or read from the file.add some kind of auto-type recognition to allow to convert data to specific types eg date, integer,extended etc.add support for locale specific recognition eg don't allow the use of the comma ',' as a delimiter if it is used as floating point delimiter. That's on the top of my head I'm have missed something. In any case is it delphi compatibility a requirement? Should we only support reading those inappropriately format fields? If yes I will revert the changes and try to implement it. ---------------------------------------------------------------------- BigChimp · 2012-09-19 13:02 taazz: yes, if you want to write a csv dataset, be my guest, but sdf is different than csv (sigh). In my opinion, the sdf dataformat is just plain )$*(%#()*$% ugly. However, interoperability with Delphi is therefore (I think) about the only reason anybody would want this abomination. I would definitely try to support everything a normal Delphi app would spit out - suggested: with strictdelimiter:=false, as that is the default. Unfortunately, because of lack of a better alternative, loads of people (including me in the beginning) insisted on using sdfdataset to load their csv files, which often works but breaks horribly on boundary conditions where - sdf specs and/or - the Delphi implementation which deviates a bit from the spec and/or - the FPC implementation, which differs rather more from the spec, see bug 19610 differ from what any sane csv format would be. Looking at the Delphi output for bug report 19610: Code: [Select]normal_string;quoted_string;"quoted;delimiter";quoted and space;"""quoted_and_starting_quote";"""quoted, starting quote, and space";quoted_with_tab character;quoted_multi line;  UnquotedSpacesInfront;UnquotedSpacesAtTheEnd   ;"  ""Spaces before quoted string""";Spaces after quoted string;   ; gives: (The numbers below indicate field number) Code: [Select]Resulting elements with strictdelimiter false: 0normal_string 1quoted_string 2quoted;delimiter 3quoted and space 4"quoted_and_starting_quote 5"quoted, starting quote, and space 6quoted_with_tab character 7quoted_multi line 8UnquotedSpacesInfront 9UnquotedSpacesAtTheEnd 10Spaces before quoted string 11Spaces after quoted string 12 Well, perhaps supporting the spaces after quoted string thing is too much. I'm almost done cleaning up the test cases to closely match the Delphi test program in 19610. I'll separate out the Spaces after quoted string case, and remove some of your added quote tests. Understand and agree with your further changes, but I think those may actually be better done in a CSV dataset. I would really like to see an RFC 4180 compliant CSV dataset and I would *strongly* suggest you take a look at combining csvdocument (see the wiki), as it's csvparser beautifully supports all the intricacies of RFC4180, as well as Excel mode etc. This means we don't need to implement a parser of our own. The rest of the dataset support could be built on this, e.g. by using memds or bufdataset or possibly ripping out the sdfdataset code (which I have my doubts about but by now you know much more about it than I). Writing out the csv to file should once again be easy as csvdocument has a class for that as well. I'll polish up the test cases for sdfdataset and post them... Awaiting with interest to hear your opinion! Thanks, BigChimp ---------------------------------------------------------------------- BigChimp · 2012-09-19 13:32 New tests: - removed some tests with quotes added by taazz that tested csv instead of sdf - use fixed directory name for output instead of asking with inputbox; although this loses functionality, it allows easier integration into fpc's dbtestframework tests - removed duplicate tests in tcsdfdata.pp and testsdfmultiline.pas from multiline. tcsdfdata will result in a patch for the dbtestframework test - added write+read test to test that fields that are saved to disks are read back resulting in the same content - changed build modes so that default build mode does not create debug files. The benefit: you can run the tests from within the ide without breaking on assertions/exceptions. If you want to debug the tests, you can run in debug mode - used the exact test data from bug 19610 to make it clearer what is expected. - edit: split out corner case as promised, uploaded newtests2.zip Discussion: I think sdfdataset should read and write sdf data created with strictdelimiter=false, because strictdelimiter is false by default and there is no option in the existing sdfdataset to change strictdelimiter. In this case, test TestDelphiCompat_DelimiterTrue is not relevant and should be commented out. (But I think it should be retained as comments in case the situation changes). Thanks, BigChimp ---------------------------------------------------------------------- taazz · 2012-09-19 13:50 Quote from: BigChimp on September 19, 2012, 03:02:54 pmtaazz: yes, if you want to write a csv dataset, be my guest, but sdf is different than csv (sigh). In my opinion, the sdf dataformat is just plain )$*(%#()*$% ugly. However, interoperability with Delphi is therefore (I think) about the only reason anybody would want this abomination. I would definitely try to support everything a normal Delphi app would spit out - suggested: with strictdelimiter:=false, as that is the default. Unfortunately, because of lack of a better alternative, loads of people (including me in the beginning) insisted on using sdfdataset to load their csv files, which often works but breaks horribly on boundary conditions where - sdf specs and/or - the Delphi implementation which deviates a bit from the spec and/or - the FPC implementation, which differs rather more from the spec, see bug 19610 differ from what any sane csv format would be. Any link for the SDF Specs? See at the bottom what I found. Quote from: BigChimp on September 19, 2012, 03:02:54 pmLooking at the Delphi output for bug report 19610: Code: [Select]normal_string;quoted_string;"quoted;delimiter";quoted and space;"""quoted_and_starting_quote";"""quoted, starting quote, and space";quoted_with_tab character;quoted_multi line;  UnquotedSpacesInfront;UnquotedSpacesAtTheEnd   ;"  ""Spaces before quoted string""";Spaces after quoted string;   ; The delimiter is ';' on this record? Quote from: BigChimp on September 19, 2012, 03:02:54 pmgives: (The numbers below indicate field number) Code: [Select]Resulting elements with strictdelimiter false: 0normal_string 1quoted_string 2quoted;delimiter 3quoted and space 4"quoted_and_starting_quote 5"quoted, starting quote, and space 6quoted_with_tab character 7quoted_multi line 8UnquotedSpacesInfront 9UnquotedSpacesAtTheEnd 10Spaces before quoted string 11Spaces after quoted string 12 Well, perhaps supporting the spaces after quoted string thing is too much. no that's the easy part understanding the sdf specs with out reading them is a bit tedious Quote from: BigChimp on September 19, 2012, 03:02:54 pm I'm almost done cleaning up the test cases to closely match the Delphi test program in 19610. I'll separate out the Spaces after quoted string case, and remove some of your added quote tests. I don't mind the spaces I do want to know the quote character used the data values in memory aka how the program sees them and on file so I can understand the problem clearly. From what I have seen so far by changing the delimiter to ';' all those lines should work as is with the current implementation. Quote from: BigChimp on September 19, 2012, 03:02:54 pm Understand and agree with your further changes, but I think those may actually be better done in a CSV dataset. I would really like to see an RFC 4180 compliant CSV dataset and I would *strongly* suggest you take a look at combining csvdocument (see the wiki), as it's csvparser beautifully supports all the intricacies of RFC4180, as well as Excel mode etc. This means we don't need to implement a parser of our own. I really don't like SDF/CSV datasets. I would never use one. They are a memory hog except if they are used for a few hundred records and their performance is terrible because they use strings as the buffer and constantly converting from and to string on each operation. I haven't looked at csvDocument my self but when it comes to csv I prefer a record parser that I can use to import the data to a more robust database this be it access/mysql/firebird or any other database engine that does not require me to load all the data in memory to work with. The only true use of a csv files is to exchange data import and export and in some extreme cases where the import might take a while eg dts service with heavy calculations before or during inserts. I had to work with csv documents on the size of a 800M to 1.4G each to load those in memory (no import mind you) would require 3x the time it took to import them. Quote from: BigChimp on September 19, 2012, 03:02:54 pmThe rest of the dataset support could be built on this, e.g. by using memds or bufdataset or possibly ripping out the sdfdataset code (which I have my doubts about but by now you know much more about it than I). Writing out the csv to file should once again be easy as csvdocument has a class for that as well. I'll polish up the test cases for sdfdataset and post them... Awaiting with interest to hear your opinion! Thanks, BigChimp I'm here to make the sdf dataset work as expected I'm not going to use it my self at all, so if no one is going to use it either I think I will just implement the delphi compatibility so every one that uses it can steel do so and leave it at that. I'm against any kind of memory dataset for anything more than a glorified memory container for extremely complex forms eg. booking tickets on a ship with various port stops in between and travel planing with linked ships, flights, trains etc, and those only for performance reasons, this means no data editing, no data inserting, no deleting except on extreme cases. While bufDataset is nice to have and it helps a lot with the SQLDB framework it needs to be as slim and fast as possible to avoid bottlenecks, I wouldn't build an un-cached dataset on top of it.  Any chance of getting my hands on delphi documentation for sdf? the following link seems to indicate that an extra file is required with the header information about the file. http://www.delphigroups.info/2/76/115068.html ---------------------------------------------------------------------- BigChimp · 2012-09-19 14:15 Quote from: taazz on September 19, 2012, 03:50:33 pmAny link for the SDF Specs? See at the bottom what I found. bug 19610 has a link to the relevant Delphi help file entry: http://docwiki.embarcadero.com/VCL/en/Classes.TStrings.DelimitedText, and del4.zip attachment in 19610 has test code that shows how Delphi (Turbo Delphi, Delphi XE) really handles sdf. Quote from: taazz on September 19, 2012, 03:50:33 pmno that's the easy part understanding the sdf specs with out reading them is a bit tedious [..] I don't mind the spaces I do want to know the quote character used the data values in memory aka how the program sees them and on file so I can understand the problem clearly. From what I have seen so far by changing the delimiter to ';' all those lines should work as is with the current implementation. See above for specs + test program. Quote from: taazz on September 19, 2012, 03:50:33 pmI really don't like SDF/CSV datasets. I would never use one. They are a memory hog except if they are used for a few hundred records and their performance is terrible because they use strings as the buffer and constantly converting from and to string on each operation. I haven't looked at csvDocument my self but when it comes to csv I prefer a record parser that I can use to import the data to a more robust database this be it access/mysql/firebird or any other database engine that does not require me to load all the data in memory to work with. Sorry, I may not have been clear: CSVDOCUMENT HAS SUCH A PARSER  Please reread my previous emails, or better, look at tcsvparser in tcsvdocument.pas (if I remember the names correctly). (Sorry, but it seems like I'm forever repeating to people that sdf is not csv, that you can use the parser or the in-memory csvdocument functionality in csvdocument etc.) Quote from: taazz on September 19, 2012, 03:50:33 pmI'm here to make the sdf dataset work as expected I'm not going to use it my self at all, so if no one is going to use it either I think I will just implement the delphi compatibility so every one that uses it can steel do so and leave it at that. Sounds good. Quote from: taazz on September 19, 2012, 03:50:33 pmAny chance of getting my hands on delphi documentation for sdf? the following link seems to indicate that an extra file is required with the header information about the file. http://www.delphigroups.info/2/76/115068.html No idea what that is, perhaps some ancient format Delphi used. The docs I linked to above don't mention those schema files. ---------------------------------------------------------------------- taazz · 2012-09-19 14:16 Quote from: BigChimp on September 19, 2012, 03:32:22 pmDiscussion: I think sdfdataset should read and write sdf data created with strictdelimiter=false, because strictdelimiter is false by default and there is no option in the existing sdfdataset to change strictdelimiter. It doesn't matter strictdelimiter is not used at all tstringlist does not format anything, everything is controlled from the sdfdataset. Even if tstringlist was responsible for the formating you can't use it to break the field values with strictdelimiter = false because it will create a new line for each space it encounters. ---------------------------------------------------------------------- BigChimp · 2012-09-19 14:27 Quote from: taazz on September 19, 2012, 04:16:45 pmIt doesn't matter strictdelimiter is not used at all tstringlist does not format anything, everything is controlled from the sdfdataset. Even if tstringlist was responsible for the formating you can't use it to break the field values with strictdelimiter = false because it will create a new line for each space it encounters. Sorry, my scenario: Delphi application writes data to sdf file using multiple tstringlist.delimitedtext FPC application reads them using sdfdataset. Does your reasoning still apply? Writing out spaces in fields with strictdelimiter off does not generate line endings AFAIR? See the test results in del4.zip ---------------------------------------------------------------------- BigChimp · 2012-09-19 14:31 Quote from: taazz on September 19, 2012, 03:50:33 pmI haven't looked at csvDocument my self but when it comes to csv I prefer a record parser that I can use to import the data to a more robust database this be it access/mysql/firebird or any other database engine that does not require me to load all the data in memory to work with. The only true use of a csv files is to exchange data import and export and in some extreme cases where the import might take a while eg dts service with heavy calculations before or during inserts. I had to work with csv documents on the size of a 800M to 1.4G each to load those in memory (no import mind you) would require 3x the time it took to import them. Let me leave you with some thoughts: 1. sqlquery uses bufdataset, right? 2. if you implement bufdataset.loadfromcsv... 3. 4. set the SELECT/INSERT SQL statment 5. mark the query dirty or whatever it's called in FPC speak, then apply updates... voila csv import into your database, right? Of course, as you mention, memory limits as things are buffered in memory. For smaller sets, quite easy perhaps. Seem to vaguely remember there is a sqlquery.savetofile/loadfromfile or something functionality... ludob posted about this regarding offline support for datasets. CSV support could tie in there However, of course you can also roll your own routines etc, Ok, off for a while. ---------------------------------------------------------------------- taazz · 2012-09-19 14:34 Quote from: BigChimp on September 19, 2012, 04:27:35 pmSorry, my scenario: Delphi application writes data to sdf file using multiple tstringlist.delimitedtext FPC application reads them using sdfdataset. Does your reasoning still apply? I don't know I have to check the del4.zip to see. does it have a file included (downloading as I type this). Quote from: BigChimp on September 19, 2012, 04:27:35 pmWriting out spaces in fields with strictdelimiter off does not generate line endings AFAIR? See the test results in del4.zip No writing out spaces does not create line endings reading them in with out quotes and strict = false does, in delphi too. EDIT: The files included seem to have delimiter = ';' not ',' have you tried the existing tests setting the delimiter = ';' before opening the file? testing as we speak. EDIT2: There is a bug in the sdfdata.pp in 1258 Code: [Select]    //Change the code below         //AnsiQuotedStr(Str, FFieldQuote);         Str := AnsiQuotedStr(Str, FFieldQuote); also the sdfdataset has a peculiar way of determining the field size and after it is opened it will not be changed so change the setup procedure lines 614..616 to Code: [Select]  TestDataset.Schema.Add('ID=20');   TestDataset.Schema.Add('NAME=100');   TestDataset.Schema.Add('BIRTHDAY=30'); ---------------------------------------------------------------------- BigChimp · 2012-09-20 05:12 Quote from: taazz on September 19, 2012, 04:34:50 pmEDIT2: There is a bug in the sdfdata.pp in 1258 Code: [Select]    //Change the code below         //AnsiQuotedStr(Str, FFieldQuote);         Str := AnsiQuotedStr(Str, FFieldQuote); Yep, that seems nasty. I'll update it in my copy here and wait for more changes Quote from: taazz on September 19, 2012, 04:34:50 pmalso the sdfdataset has a peculiar way of determining the field size and after it is opened it will not be changed so change the setup procedure lines 614..616 to Code: [Select]  TestDataset.Schema.Add('ID=20');   TestDataset.Schema.Add('NAME=100');   TestDataset.Schema.Add('BIRTHDAY=30'); Ok. Latest version of my files can always be downloaded from https://bitbucket.org/reiniero/fpc_laz_patch_playground/src directory sdfdataset Thanks, BigChimp ---------------------------------------------------------------------- BigChimp · 2012-09-20 08:45 Sorry, correction to test code - had overlooked some spaces: sdfdataset test: correction: corner case strict delimiter true has extra spaces in the first empty line Committed to repository Also removed some ;s at the end of test cases (which would lead to creation of a new field) but that has no influence on the tests as they were. ATM writing compiler test cases for tstringlist.delimitedtext to make sure fpc has the right test cases to work against in fixing .delimitedtext/sdf support. Checking test case output with Turbo Delphi 2006. Test case (work in progress) can be found in tw19610.pp in repository (root directory) Edit: got it working; all tests run on Turbo Delphi 2006; opening new thread for feedback requests. ---------------------------------------------------------------------- taazz · 2012-09-20 16:59 a small update. I'm working on the space's (leading and trailing) problem at the moment. I'm trying various methods to keep them with out much success so far. I'll try to post something new in the weekend no promises though. ---------------------------------------------------------------------- taazz · 2012-09-22 21:05 Please Find attached the latest version. In this version I have rewrote the field parser. It now tries to guess if there is a quoted value in a field and if it finds one it removes the quotes preserving all the data outside the quotes (before and after). Curently there is no way to get back only the data inside the quote unless the data outside the quotes are only spaces and you have selected to trim them. Changed the record initialization process to fill the empty space with #1 instead of #32(' ') making it easier to distinguish between record empty space and spaces that are part of the field's data. Introduced 2 new properties TrimTrailingSpaces & TrimLeadingSpaces. The default behavior is not to trim anything. The current implementation preserves all the data in a file allowing the end user to delete or trim what ever he wants to. Didn't had the time to change how a field's value is quoted when saving yet, as far as I can see if any non standard characters are found in the value they are quoted. Regards Jo. PS. BC you think this is good enough? ---------------------------------------------------------------------- BigChimp · 2012-09-23 15:24 Sorry, Jo, I've been busy. I might have a look at it today or in a day or two. Have you run the test suite (https://bitbucket.org/reiniero/fpc_laz_patch_playground/src, sdfdataset,testgui.lpi) against your changes? I think that would be my yardstick to see whether it has improved.... Will get back to you... ---------------------------------------------------------------------- BigChimp · 2012-09-24 05:15 Uploaded tests zip for convenience: https://bitbucket.org/reiniero/fpc_laz_patch_playground/downloads/sdfd ataset_tests_24Sept2012.zip New tests as always are very welcome. Please see the readme.txt for details. Notes: 1. Noticed gui testrunner Laz trunk+FPC trunk just hung on test run. Added some try..except around writelns that seemed to solve things. Test results: 1. trunk sdfdata: 4 failures: TestDelphiCompat_DelimiterFalse TestDelphiCompat_DelimiterFalseCornerCases TestDelphiCompat_DelimiterTrue TestDelphiCompat_DelimiterTrueCornerCases 2. taazz sdfdata: 3 failures: TestDelphiCompat_DelimiterFalse now works. The failure in TestDelphiCompat_DelimiterTrue is on a different string than the the trunk failure Delimiter false is the default mode, so that is a big win. Result XML file and the more useful console output lines with detailed differences are attached to the tests download mentioned above. Remarks: 1. taazz, perhaps renaming maxintlength etc to a clearer name (SDFDataMaxInt or something) may be a good idea? Otherwise people may perhaps mistake it for a RTL defined constant? 2. TestDelphiCompat_DelimiterFalseCornerCases gives data containing "TRUE" now!?! See console output file. Perhaps you need to fiddle with TrimLeadingSpaces for TestDelphiCompat_DelimiterFalseCornerCases, TestDelphiCompat_DelimiterTrueCornerCases. Could that help? In general, I'm not worried very much about the cornercases tests as those are just too weird. It would be nice if it could work for delimitertrue as well, but I suppose the code as is is already a major improvement, because delimiter=false is the default in Delphi. Finally, it's not up to me but to the developers whether they'll commit it... but having a documented set of improvements on test cases is a very good way of proving our point that a patch makes sense. Thanks, BigChimp ---------------------------------------------------------------------- BigChimp · 2012-09-24 15:56 @taazz: mailng list thread on Delphi compatibility http://lists.freepascal.org/lists/fpc- pascal/2012-September/034957.html ... apparently it's not necessary!?!? I'll wait for confirmation. If so, I'll remove the test code. I only wanted to improve sdfdata so it can better interoperate with sdf data. If that is not required, I don't see the need for the test code. Thanks, BigChimp ---------------------------------------------------------------------- taazz · 2012-09-24 16:43 Hi FC, First I will like to thank you for all you support. Your test cases where impressive. This think started as a simple task (multiline support for the records) and ended rewriting the record parser to support a lot more. This is the final enhancement on the parsing that I'm going to do. It now passes all the tests except the one called TestDelphiCompat_DelimiterTrueCornerCases although it can parse the 1st field correctly it will never parse the rest as delphi does. After the changes I've made, I made sure that the space character is not considered a break character anywhere, they can be trimmed  if the user selects to, but that's about it. Find attached the latest changes in this one I have introduced a new property AlwaysDeQuote when true the parser will remove the quotes regardless of the their position in the string, if false it will remove them only if the string starts with a quote. And this is the default behavior for CSV files. The quote remover is smart enough to not loose any data when parsing and unquote multiple quoted values in the string correctly. I don't think that I should try to support the last case it will never be used and I don't think it is desirable either. ---------------------------------------------------------------------- BigChimp · 2012-09-25 09:18 Hi Jo, Thanks for the kind words & persevering. Recent discussions on the mailing list seem to indicate sdfdataset should be RFC4180 CSV compliant... which is at least a much better format, I think. So a lot of the test suite can be discarded Regards, BigChimp ---------------------------------------------------------------------- taazz · 2012-09-25 14:44 Well I think that the current implementation is fully RFC compliant I'm going  to sharpen my Testing skills and see if I can write a couple of test to prove that. I'm going to use some of your tests as well and in the end I'm going to uploaded on the bug tracker along with the tests if they don't want to include it with the official distribution that's their choice. Best regards Jo. ---------------------------------------------------------------------- BigChimp · 2012-10-12 15:34 Well, we can use RFC4180 for sdfdataset. As long as there are test cases that cover RFC4180 (or a big part of it) I'd be happy to look into getting it into FPC trunk.... so if you are still interested in writing them, I'm definitely interested in getting the changes committed. Thanks, BigChimp ---------------------------------------------------------------------- taazz · 2012-10-12 17:11 Sorry BC, I have it on my todo list for sometime now but I've been busy converting various components from delphi for my own applications so it will happen eventually but not at the moment. I have spend almost a week already on that component correcting and changing things it would be a waste of my time to stop now so definitely will do it at some point. ---------------------------------------------------------------------- BigChimp · 2012-10-12 17:34 Got it, taazz, I'll keep the bug reports open for you Thanks, BigChimp