search

mafintosh / csv-parser

Streaming csv parser inspired by binary-csv that aims to be faster than everyone else
MIT License
1.41k stars 134 forks source link

fix: losing all headers after a broken line #151

Closed aheissenberger closed 3 years ago

aheissenberger commented 4 years ago

This PR contains:

Breaking Changes?

If yes, please describe the breakage.

when amount of columns of cell not equal to amount of headers the object returned is using indexed keys instead of the headers. old version did only replace headers with index keys when there where more columns. When there where less columns the headers where still used.

Please Describe Your Changes

Problem #150

codecov-io commented 4 years ago

Codecov Report

Merging #151 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
+ Coverage   97.24%   97.26%   +0.01%     
==========================================
  Files           1        1              
  Lines         145      146       +1     
==========================================
+ Hits          141      142       +1     
  Misses          4        4
Impacted Files Coverage Δ
index.js 97.26% <100%> (+0.01%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 264e15a...240d075. Read the comment docs.

aheissenberger commented 4 years ago

In an older version of this parser the logic for broken rows did not have any impact on the header mapping. What was the reason to change the behaviour to switch to indexed object if a line is broken?

shellscape commented 4 years ago

@aheissenberger that was likely a regression or untested side-effect when the indexed headers were added due to incomplete tests or poor coverage.

aheissenberger commented 4 years ago

@shellscape using indexed props for columns with wrong amount of columns broke my code which was based on an older version of css-parser. I changed the code to be more consistent and the headers are now always set except for the case of more columns - here I will use the index for columns with no existing header.

SimonSimCity commented 4 years ago

We've encountered the same problem and can confirm that this PR (as it is now) will fix the problem. Currently we're back on 2.3.0 until a version containing this fix is released.

nathsimpson commented 4 years ago

@mafintosh When can this be released?

louisgv commented 4 years ago

@shellscape @mafintosh would love to see this fix merged and published

shellscape commented 4 years ago

I won't have time to address any issues or PRs for this project until mid to late January, unfortunately.

MalxMalx commented 4 years ago

Hi, don't want to be seen as unpolite, but can you merge this PR? It's already February. I understand that you may have other important things on your table, but this problem affects my project, so I hope it can be fixed soon.

SimonSimCity commented 4 years ago

@shellscape if you feel comfortable with the idea, I volunteer for helping you out with this project. I care very much about this project and want it to remain both healthy, in good shape and code-quality.

shellscape commented 4 years ago

@MalxMalx that's why it's open source. please use the fork this PR was based on.

@SimonSimCity not up to me unfortunately

joshkay commented 4 years ago

Also looking to have this merged. Updated this dependency and this bug makes it unusable for me currently. Downgrading to 2.3.0 where it was originally working for the time being. @shellscape @SimonSimCity would love to help you get this merged if possible

stefan-jonasson commented 3 years ago

I'm also waiting for this to be merged, anything I can do to help?