Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
FourierTransformer opened this issue May 31, 2023 · 7 comments
Closed

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

FourierTransformer opened this issue May 31, 2023 · 7 comments

Comments

@FourierTransformer
Copy link
Owner

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 FourierTransformer changed the title Can't call ftcsv.parse twice in a row in the REPL Can't call ftcsv.parse with a string twice in a row May 31, 2023
@FourierTransformer
Copy link
Owner Author

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)

@timothymtorres
Copy link

timothymtorres commented May 31, 2023

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.

@FourierTransformer
Copy link
Owner Author

FourierTransformer commented May 31, 2023

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.

@timothymtorres
Copy link

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.

@FourierTransformer
Copy link
Owner Author

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

@timothymtorres
Copy link

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.

@FourierTransformer
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants