Skip to content

Conversation

scossu
Copy link

@scossu scossu commented Sep 22, 2025

This small PR allows encoding CSV from data entirely missing a field that was indicated in the fieldsToKeep option.

Useful when encoding multiple data sets into similarly formatted CSV files.

>  csv = require "ftcsv"
>  data = {
>> {a = 1, b = 2, c = 3},
>> {a = 10, b = 20},
>> {a = 100, c = 200},
>> }
>  csv.encode(data, {fieldsToKeep = {"a", "b", "c", "d"}})
/home/ste/.luarocks/share/lua/5.4/ftcsv.lua:757: ftcsv: the field 'd' doesn't exist in the inputTable

Stack trace:
	#0 [C]: in function ?
	#1 [C]: in function 'error'
	#2 /home/ste/.luarocks/share/lua/5.4/ftcsv.lua:757: in function 'validateHeaders'
	#3 /home/ste/.luarocks/share/lua/5.4/ftcsv.lua:817: in function 'initializeGenerator'
	... tail calls
	#4 /home/ste/.luarocks/share/lua/5.4/ftcsv.lua:827: in function ?
	#5 [C]: in function 'enter'
	#6 ...luarocks/lib/luarocks/rocks-5.4/luaprompt/0.9-1/bin/luap:230: in the main chunk
	#7 [C]: in function ?

>  csv.encode(data, {fieldsToKeep = {"a", "b", "c", "d"}, allowEmpty = true})
_[2] = [[
"a","b","c","d"
"1","2","3","nil"
"10","20","nil","nil"
"100","nil","200","nil"
]]
>  

@FourierTransformer
Copy link
Owner

Ahhh yeah, that makes sense and looks good overall! I think maybe it should be changed to allowMissingKeys instead of allowEmpty to try and make it less ambiguous (open to other suggestions as well).

Oh and maybe add a few unit tests. I'm assuming it also works with the encodeNilAs option?

@scossu
Copy link
Author

scossu commented Sep 23, 2025

In my implementation it works with EncodeNilAs. I agree on renaming. I will do that and add some tests in the next day or two. Thanks.

@scossu
Copy link
Author

scossu commented Sep 25, 2025

I added a unit test; it only runs on a dedicated CSV/JSON pair because otherwise it breaks simple_crlf. Other tests are failing as well, but they don't seem related to this PR.

@FourierTransformer
Copy link
Owner

FourierTransformer commented Sep 29, 2025

All the tests pass on the main branch: https://github.com/FourierTransformer/ftcsv/actions/runs/17284534932/job/49059071095. I have no intention of changing existing behaviour, and no plans for a major release, so at the moment there can't be any breaking changes to exisitn tests. You may want to look into what's causing the failures. Also, I think you may have accidentally included other new options noHeader and noData.

@scossu
Copy link
Author

scossu commented Sep 29, 2025

Oops, I'm sorry, those were options I was experimenting with and that weren't completed. I will remove them.

@scossu
Copy link
Author

scossu commented Oct 6, 2025

Tests pass now. Is anything else missing for approving this PR?

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

Successfully merging this pull request may close these issues.

2 participants