Added optional argument to contains_header to skip multiple header lines #22
Conversation
Some banks add initial lines to the csv file with for example your starting balance. This option allows one to skip these lines
| opts.on("", "--contains-header", "The first row of the CSV is a header and should be skipped") do |contains_header| | ||
| options[:contains_header] = contains_header | ||
| opts.on("", "--contains-header [N]", "The first row of the CSV is a header and should be skipped. Optionally add the number of rows to skip.") do |contains_header| | ||
| options[:contains_header] = contains_header.to_i || 1 |
There was a problem hiding this comment.
I think this might always be 0 if N isn't provided because nil.to_i is 0 and 0 || 1 is 0. Did you try it?
There was a problem hiding this comment.
Also, did you try it and see that, if --contains-header is skipped, it does nothing, if --contains-header is included, it does 1, and if --contains-header N is included, it does N? Just sanity checking! :)
|
Thanks @BlackEdder! |
Fixes bug since nil.to_i is 0
|
You were right. nil.to_i indeed solves to 0. I now tested all three cases (not provided, provided without argument and provided with argument) to check if they worked correctly and they did. Sorry about not checking properly before :( (I thought I had, but added the to_i after checking the first two options and then didn't recheck). |
|
That's why we code review :) Thanks for your contribution! |
Added optional argument to contains_header to skip multiple header lines
Some banks add initial lines to the csv file with for example your
starting balance. This option allows one to skip these lines
This patch supersedes pull request #21