feat(csv): add parseLine() convenience for single-line CSV records (refs #3765)#7114
feat(csv): add parseLine() convenience for single-line CSV records (refs #3765)#7114MukundaKatta wants to merge 1 commit intodenoland:mainfrom
Conversation
…ecords Adds a small synchronous helper that takes one CSV line and returns string[]. Useful when callers already have line-split input and don't need full document/stream semantics. Refs: denoland#3765
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7114 +/- ##
=======================================
Coverage 94.61% 94.61%
=======================================
Files 633 633
Lines 51777 51790 +13
Branches 9324 9330 +6
=======================================
+ Hits 48987 49000 +13
Misses 2216 2216
Partials 574 574 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bartlomieju
left a comment
There was a problem hiding this comment.
Thanks for the PR! Appreciate the thorough tests and documentation.
However, this needs significant rework. The main concern is that this doesn't match issue #3765's intent. That issue asks for parseLine as an internal primitive that parse() and CsvParseStream build on top of — a simplification/refactoring of the CSV internals. This PR does the opposite: it wraps the existing Parser class as an external convenience. It doesn't simplify anything internally; it just adds API surface area.
See inline comments for the implementation-level issues.
| } | ||
|
|
||
| /** Options for {@linkcode parseLine}. */ | ||
| export interface ParseLineOptions { |
There was a problem hiding this comment.
ParseLineOptions manually re-declares separator, trimLeadingSpace, and lazyQuotes — fields that already exist on ReadOptions. This creates a maintenance burden (if ReadOptions changes, this must stay in sync). If this moves forward, this should be Pick<ReadOptions, "separator" | "trimLeadingSpace" | "lazyQuotes"> instead.
| line: string, | ||
| options: ParseLineOptions = {}, | ||
| ): string[] { | ||
| const stripped = line.startsWith(BYTE_ORDER_MARK) ? line.slice(1) : line; |
There was a problem hiding this comment.
This BOM stripping is redundant — Parser.parse() already strips the BOM internally (line 208). The double-strip is harmless but suggests the internals weren't fully reviewed.
| ? stripped.slice(0, -2) | ||
| : stripped.endsWith("\n") || stripped.endsWith("\r") | ||
| ? stripped.slice(0, -1) | ||
| : stripped; |
There was a problem hiding this comment.
This trailing newline stripping is also redundant — Parser.#readLine() already treats \r\n, \n, and \r as line terminators. A trailing newline just produces an empty second record, which Parser.parse() drops (empty records are not pushed to the result array).
| : stripped.endsWith("\n") || stripped.endsWith("\r") | ||
| ? stripped.slice(0, -1) | ||
| : stripped; | ||
| const parser = new Parser(options); |
There was a problem hiding this comment.
This allocates a new Parser instance on every call. For a function named parseLine that users will naturally reach for in a loop over pre-split lines, this is wasteful — the constructor sets up options, and .parse() runs separator validation each time.
| * @param options Parsing options. | ||
| * @returns The fields parsed from the line. | ||
| */ | ||
| export function parseLine( |
There was a problem hiding this comment.
New public APIs in std typically go through an unstable phase with an @experimental tag. This adds a stable API directly — should either be marked @experimental or get explicit maintainer sign-off to ship stable.
|
@bartlomieju thanks for the detailed review. You're right — this missed the actual ask in #3765. The right shape is Closing this and will open a fresh PR once the refactor is sketched out properly. Inline points (Pick on options, redundant BOM/newline strips, allocating Parser per call, @experimental) all noted for the next pass. |
Summary
Refs #3765. Adds
parseLine(line: string, options?: ParseLineOptions): string[]— a small synchronous convenience for single-record CSV input. Defaults to comma separator, no trim, strict quotes. Strips leading BOM and a single trailing CR/LF/CRLF.API
Documents the trade-off vs full
parse()—parseLinedoesn't continue multi-line quoted fields, doesn't validate field count across records, and doesn't honorcommentlines. For those useparse.Files changed
csv/parse.ts— newParseLineOptionsinterface andparseLine()function. Reuses internalParserclass. No breaking changes to existingparse()signature.csv/parse_test.ts— 12 new sub-steps covering basic parsing, quoting, escaped quotes, custom separator (TSV),trimLeadingSpace,lazyQuotes, trailing newline strip, BOM strip, error case, and return-type assertion.Test plan
deno fmt,deno lint,deno check,deno testall clean