ftcsv icon indicating copy to clipboard operation
ftcsv copied to clipboard

Can't call ftcsv.parse with a string twice in a row

Open FourierTransformer opened this issue 2 years ago • 7 comments

I ran into this in the REPL while looking at #37

Gonna see if it affects actual scripts

> ftcsv = require "ftcsv"
> options = {loadFromString=true, ignoreQuotes=true}
> ftcsv.parse('a,b,c\n"apple,banana,carrot', ",", options)
> ftcsv.parse('a,b,c\n"apple,banana,carrot', ",", options)
./ftcsv.lua:448: ftcsv: bufferSize can only be specified using 'parseLine'. When using 'parse', the entire file is read into memory
stack traceback:
	[C]: in function 'error'
	./ftcsv.lua:448: in function 'parseOptions'
	./ftcsv.lua:542: in function 'parse'
	stdin:1: in main chunk
	[C]: ?

FourierTransformer avatar May 31 '23 03:05 FourierTransformer

Seems to affect things loaded by string, but not things loaded via files.

So this will cause an error:

local options = {loadFromString=true, ignoreQuotes=true}
local a = ftcsv.parse('a,b,c\n"apple,banana,carrot', ",", options)
print(a[1].a)
a = ftcsv.parse('a,b,c\n"apple,banana,carrot', ",", options)
print(a[1].a)

but this wont:

local a = ftcsv.parse("ALLUSERS.csv", ",")
print(a[1].Name)
a = ftcsv.parse("ALLUSERS.csv", ",")
print(a[1].Name)

FourierTransformer avatar May 31 '23 03:05 FourierTransformer

I ran into the same problems when I was performing unit tests on ftcsv in a private repo. Some of the libraries required by ftcsv had their versions bumped higher and that MIGHT be the cause of the problem.

timothymtorres avatar May 31 '23 03:05 timothymtorres

I think it happens because ftcsv modifies the original options table and adds in a value for bufferSize:

if options.bufferSize == nil then
	    options.bufferSize = 2^16

so when the options table gets used again, it is then present. Interesting. I guess I've never ran into it in use because I usually inline the options table instead of re-using it. I mostly did it in the docs to make it easier to read. I'll probably play arond with all of this some more tomorrow and see what can be done.

FourierTransformer avatar May 31 '23 03:05 FourierTransformer

The repo I was using was a direct c/p of all the code with no modifications and it was still failing unit tests. At first it was due to GitHub switching all new repos to use main branch instead of master which broke GitHub Actions since it was pointing to master. After I fixed this, I was getting the same problem you are detailing here, which shouldn't be happening since there were no other changes. The only thing I can think of is the required libraries bumping to a higher version.

I also added you as a collaborator so you can see for yourself.

timothymtorres avatar Jun 01 '23 03:06 timothymtorres

Yeah, it seemed to be the buffserSize value being set. I re-ordered the logic a bit in the argument parsing, so bufferSize wont get set when using parse by itself, and wrote some unit tests around it.

The code is in https://github.com/FourierTransformer/ftcsv/tree/optional-delimiter if you want to check it out! It also has the changes from #37

FourierTransformer avatar Jun 02 '23 00:06 FourierTransformer

Looks good!

I am planning to work on beefing up your CI/CD with GitHub Actions so it runs smoother for the repo. Going to add a linter and automated doc generator via code comments. Expect several new pull requests over this week. If you have any questions, feel free to ping me here or on discord.

timothymtorres avatar Jun 02 '23 11:06 timothymtorres

As nice as that offer is, I'm good with the setup that I have. It works for me as a single dev and I like the minimalism. Since this isn't a repo with a lot of activity (mostly just the odd bugfix or feature here and there), and I'm planning to support this indefinitely, I need to be able to maintain it with a low level of friction.

FourierTransformer avatar Jun 05 '23 03:06 FourierTransformer