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

data or record deprecated? #73

Closed
chrisvariety opened this issue Oct 20, 2014 · 9 comments
Closed

data or record deprecated? #73

chrisvariety opened this issue Oct 20, 2014 · 9 comments

Comments

@chrisvariety
Copy link

The parser stream code clearly emits record: https://github.com/C2FO/fast-csv/blob/master/lib/parser/parser_stream.js#L192-L197

And the tests clearly all use record, e.g. https://github.com/C2FO/fast-csv/blob/master/test/fast-csv.test.js#L461 (or seem to use some weird mix of data and record)

But the Readme notes that record is deprecated: https://github.com/C2FO/fast-csv/blame/master/README.md#L37

So I'm confused whether the emitRecord function in parser_stream is incorrect or the Readme?

@chrisvariety chrisvariety changed the title data data or record deprecated? Oct 20, 2014
@chrisvariety
Copy link
Author

The reason I'm asking is because this code does not work at all:

csv.fromPath(process.argv[2], {
  trim: true,
  headers: true,
  ignoreEmpty: true
}).on('error', function() {
  console.log('error', arguments);
}).on('data', function(data){
  console.log('DATA', data);
}).on('end', function() {
  console.log('done');
});

But this code works fine:

csv.fromPath(process.argv[2], {
  trim: true,
  headers: true,
  ignoreEmpty: true
}).on('error', function() {
  console.log('error', arguments);
}).on('record', function(data){
  console.log('DATA', data);
}).on('end', function() {
  console.log('done');
});

@doug-martin
Copy link
Contributor

What version are you using?

@chrisvariety
Copy link
Author

0.5.2!

@doug-martin
Copy link
Contributor

What node version?

@chrisvariety
Copy link
Author

0.11.14 -- that could be the problem if you are transforming the record emit into data somewhere.

@doug-martin
Copy link
Contributor

This is going to take a little digging it works great on v0.10 but I have not tried it on 11...Ill dig in a little more and let you know what I find

@doug-martin
Copy link
Contributor

Ok after a little digging the streams pause/resume functions have been completely redone so you dont have to override them to get pause/resume functionality. Working on a fix.

Should note it was the implementing code that caused your stream not to go into flowing mode and therefore the data events were not emitted

doug-martin added a commit to doug-martin/fast-csv that referenced this issue Oct 21, 2014
* Fixed issues with v0.11 stream implementation C2FO#73
* Fixed issues with pause/resume and data events in v0.10 C2FO#69
* Fixed the double invoking of done callback when parsing files C2FO#68
* Refactored tests
@doug-martin doug-martin mentioned this issue Oct 21, 2014
@doug-martin
Copy link
Contributor

Fixed with tests as of v0.5.3

@chrisvariety
Copy link
Author

Thanks so much! This version works great--

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