-
Notifications
You must be signed in to change notification settings - Fork 540
fix(data-grid): apply improvements from code review findings #1118
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
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
4181bf1
Port North's improved TSV parser
sadmann7 c6448e4
fix(data-grid): apply fixes discovered during North port
sadmann7 e2c4a52
chore: rebuild registry
sadmann7 4db65fc
test: update multiline paste test to use quoted TSV format
sadmann7 571a3ca
feat(data-grid): add hybrid TSV parser for both quoted and unquoted m…
sadmann7 f6778d6
refactor(data-grid): consolidate TSV parsing utilities
sadmann7 494eb2d
refactor(data-grid): simplify TSV parser to single function
sadmann7 3910b01
chore: rebuild registry
sadmann7 27e39d1
fix(data-grid): improve TSV parser detection for quoted fields
sadmann7 efaff80
fix(data-grid): handle multiline content in any column when pasting
sadmann7 38f927f
perf(data-grid): track buffer tab count instead of re-scanning
sadmann7 687da50
refactor(data-grid): inline hasQuotedFields check
sadmann7 ea0ccc0
test(data-grid): add unit tests for parseTsv edge cases
sadmann7 be267a1
chore: rebuild registry
sadmann7 69e5624
merge: combine fix-issues-from-north-port into single PR
sadmann7 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { parseTsv } from "@/lib/data-grid"; | ||
|
|
||
| describe("parseTsv", () => { | ||
| describe("basic parsing", () => { | ||
| it("should parse simple single-row TSV", () => { | ||
| expect(parseTsv("Alice\tKickflip\t95", 3)).toEqual([ | ||
| ["Alice", "Kickflip", "95"], | ||
| ]); | ||
| }); | ||
|
|
||
| it("should parse multiple rows", () => { | ||
| expect(parseTsv("Alice\tKickflip\t95\nBob\tOllie\t88", 3)).toEqual([ | ||
| ["Alice", "Kickflip", "95"], | ||
| ["Bob", "Ollie", "88"], | ||
| ]); | ||
| }); | ||
|
|
||
| it("should handle single-column paste", () => { | ||
| expect(parseTsv("Alice\nBob\nCharlie", 1)).toEqual([ | ||
| ["Alice"], | ||
| ["Bob"], | ||
| ["Charlie"], | ||
| ]); | ||
| }); | ||
|
|
||
| it("should skip empty rows", () => { | ||
| expect(parseTsv("Alice\tKickflip\t95\n\nBob\tOllie\t88", 3)).toEqual([ | ||
| ["Alice", "Kickflip", "95"], | ||
| ["Bob", "Ollie", "88"], | ||
| ]); | ||
| }); | ||
| }); | ||
|
|
||
| describe("quoted fields (standard TSV)", () => { | ||
| it("should handle quoted multiline content", () => { | ||
| const text = | ||
| 'Alice\tKickflip\t95\nBob\t"Trick with\nmultiple\nlines"\t98'; | ||
| expect(parseTsv(text, 3)).toEqual([ | ||
| ["Alice", "Kickflip", "95"], | ||
| ["Bob", "Trick with\nmultiple\nlines", "98"], | ||
| ]); | ||
| }); | ||
|
|
||
| it("should handle escaped quotes", () => { | ||
| const text = '"She said ""hello"""\t42'; | ||
| expect(parseTsv(text, 2)).toEqual([['She said "hello"', "42"]]); | ||
| }); | ||
|
|
||
| it("should handle Windows line endings", () => { | ||
| const text = '"Line 1\r\nLine 2"\tvalue'; | ||
| expect(parseTsv(text, 2)).toEqual([["Line 1\r\nLine 2", "value"]]); | ||
| }); | ||
|
|
||
| it("should handle mixed quoted and unquoted fields", () => { | ||
| const text = 'plain\t"quoted\nfield"\t123'; | ||
| expect(parseTsv(text, 3)).toEqual([["plain", "quoted\nfield", "123"]]); | ||
| }); | ||
| }); | ||
|
|
||
| describe("unquoted multiline (tab counting)", () => { | ||
| it("should handle multiline in last column", () => { | ||
| const text = "Alice\tKickflip\t95\nBob\tTrick with\nmultiple\nlines\t98"; | ||
| expect(parseTsv(text, 3)).toEqual([ | ||
| ["Alice", "Kickflip", "95"], | ||
| ["Bob", "Trick with\nmultiple\nlines", "98"], | ||
| ]); | ||
| }); | ||
|
|
||
| it("should handle multiline in middle column", () => { | ||
| const text = | ||
| "Alice\tShort note\t95\nBob\tLine 1\nLine 2\nLine 3\t88\nCharlie\tSimple\t77"; | ||
| expect(parseTsv(text, 3)).toEqual([ | ||
| ["Alice", "Short note", "95"], | ||
| ["Bob", "Line 1\nLine 2\nLine 3", "88"], | ||
| ["Charlie", "Simple", "77"], | ||
| ]); | ||
| }); | ||
|
|
||
| it("should handle multiple rows with multiline in middle columns", () => { | ||
| const text = [ | ||
| "Alice\tShort\t1", | ||
| "Bob\tMulti", | ||
| "line", | ||
| "content\t2", | ||
| "Charlie\tAnother", | ||
| "multi\t3", | ||
| "Dave\tPlain\t4", | ||
| ].join("\n"); | ||
| expect(parseTsv(text, 3)).toEqual([ | ||
| ["Alice", "Short", "1"], | ||
| ["Bob", "Multi\nline\ncontent", "2"], | ||
| ["Charlie", "Another\nmulti", "3"], | ||
| ["Dave", "Plain", "4"], | ||
| ]); | ||
| }); | ||
| }); | ||
|
|
||
| describe("data with JSON values (no false positives)", () => { | ||
| it("should use tab counting when quotes are inside field values not delimiters", () => { | ||
| const text = 'Alice\t["React","Node.js"]\t95\nBob\t["Python"]\t88'; | ||
| expect(parseTsv(text, 3)).toEqual([ | ||
| ["Alice", '["React","Node.js"]', "95"], | ||
| ["Bob", '["Python"]', "88"], | ||
| ]); | ||
| }); | ||
|
|
||
| it("should handle JSON values with unquoted multiline", () => { | ||
| const text = [ | ||
| 'Alice\tShort note\t["React"]\t1', | ||
| "Bob\tLine 1", | ||
| 'Line 2\t["Python"]\t2', | ||
| 'Charlie\tPlain\t["SQL"]\t3', | ||
| ].join("\n"); | ||
| expect(parseTsv(text, 4)).toEqual([ | ||
| ["Alice", "Short note", '["React"]', "1"], | ||
| ["Bob", "Line 1\nLine 2", '["Python"]', "2"], | ||
| ["Charlie", "Plain", '["SQL"]', "3"], | ||
| ]); | ||
| }); | ||
| }); | ||
|
|
||
| describe("edge cases", () => { | ||
| it("should return empty array for empty string", () => { | ||
| expect(parseTsv("", 0)).toEqual([]); | ||
| }); | ||
|
|
||
| it("should handle single cell", () => { | ||
| expect(parseTsv("hello", 1)).toEqual([["hello"]]); | ||
| }); | ||
|
|
||
| it("should fallback to simple split when no tabs detected", () => { | ||
| expect(parseTsv("line1\nline2\nline3", 1)).toEqual([ | ||
| ["line1"], | ||
| ["line2"], | ||
| ["line3"], | ||
| ]); | ||
| }); | ||
| }); | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.