-
Notifications
You must be signed in to change notification settings - Fork 15
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
[FIX] Cannot parse CSV with newlines #642
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work 🎉
@@ -2,7 +2,10 @@ | |||
// | |||
// SPDX-License-Identifier: AGPL-3.0-only | |||
|
|||
import { parseString as parseStringAsCsv } from '@fast-csv/parse'; | |||
// eslint-disable-next-line unicorn/prefer-node-protocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I asked this before, why not just add the node protocol?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
importing node:assert
gives me this linter error:
'node:assert' import is restricted from being used. Please use the library `assert` instead to keep browser compatibility. You might need to disable rule `unicorn/prefer-node-protocol` to do so no-restricted-imports`
(reason) => { | ||
assert(typeof reason === 'string'); | ||
return R.err({ | ||
message: reason, | ||
diagnostic: { | ||
node: context.getCurrentNode(), | ||
property: 'name', | ||
}, | ||
}); | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow I didn't know that you could add a second parameter to .then
:D I still prefer using .catch
because it think has more semantic value, but I don't mind using it.
I was wondering if we should add improvements for selecting just the first couple of rows. I think it's a very common use case, and removing the first couple of rows of a large text file could lead to performance issue (if I understood the algorithm correctly). |
Closes #608.
Problem: The
TextFile
IOType contains a list of lines, whichCSVInterpreter
considers a string value containing a newline as two different rows.Solution: Change
TextFile
to contain a single string, not a list of lines. This means that existing blocks relying on the lines being already split have been adapted as well. E.g. theTextLineDeleter
splits the string into lines, deletes the lines required and joins the lines back into a single string.