introduce support for jsonlines input files

[s4ke]

implements #498

If you are fine with this, I can add the necessary unit tests as well.

[blgm]

Hi @s4ke, thank you for this. I have a couple of thoughts:

1. As far as I can see "jsonlines" and "ndjson" are essentially the same thing. I don't have an opinion on which name is best, but I think it makes jfq easier to understand if we make it clear that they are the same (if that is indeed correct), and I think we could make that clear in the options descriptions like this:

.option('-n, --ndjson', 'JSON Lines Output (also known as Newline Delimited JSON)')
...
.option('-l, --jsonlines', 'JSON Lines Input (also known as Newline Delimited JSON)')

What do you think? Does that make it clearer?

2. I'd like to see at least one test for this, probably in the file src/__tests__/jfq_inputs.js. I know that people have different opinions on testing. The reason I'd like one is that I don't run any manual tests before creating a new version of jfq, and I don't want it to be accidentally broken by a future change to the code or to one of the dependencies.

How would you feel about making those changes? Or do you have another perspective, in which case I'd be interesting in hearing it?

[s4ke]

Hey @blgm,

1. Yes, they are indeed the same, but since I did not want to break ndjson in an backwards incompatible way by enforcing the input to be new line delimited json as well I opted for the separate flag. Tbh, something like --input-ndjson and --output-ndjson (or the counterpart with jsonlines in the name) should probably be better?

2. sure. I just wanted to check in whether this is something you would be willing to add to this lib. And yes, I totally agree with you, this needs more tests!

[s4ke]

What do you think about the naming changes? Should we differentiate between input and output in the flags (while still allowing ndjson as an alias to --output-jsonlines)?

[blgm]

Should we differentiate between input and output in the flags (while still allowing ndjson as an alias to --output-jsonlines)?

Yes, I like this idea because it doesn't break anyone, but provides consistency for any new users

I'd be very happy to merge this PR and create a new version if you're able to make the changes discussed above.

[blgm]

Hi @s4ke. I just wanted to check in on where this PR is. I haven't looked at this yet as the unit tests are failing, so had assumed it was still a work in progress. What are your thoughts?

[s4ke]

Hey @blgm ,

Sorry, I have not had the time to finish this. Last thing i remembered, was that I had trouble getting the unit test harness to run with my code.

We are actually already using the code from our fork in some projects though. No problems so far. Streaming data works like a charm.

[blgm]

There's no rush @s4ke.