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

Addition of CSV #432

Merged
merged 3 commits into from May 24, 2019

Conversation

2 participants
@zekth
Copy link
Contributor

commented May 22, 2019

Currently work in progress.
Ported tests from golang

EDIT:
Remaining quotes and CRLF tests to fix and we're done.

@zekth zekth force-pushed the zekth:csv branch from 3dba539 to f97e2f0 May 22, 2019

@@ -0,0 +1,2 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
import "./mod_test.ts";

This comment has been minimized.

Copy link
@ry

ry May 23, 2019

Contributor

Why not encoding/csv_test.ts ?

@@ -0,0 +1,139 @@
import { BufReader, BufState } from "../../io/bufio.ts";

This comment has been minimized.

Copy link
@ry

ry May 23, 2019

Contributor

Rename file to encoding/csv.ts

Please add

// Ported from Go:
// https://github.com/golang/go/blob/go1.12.5/src/encoding/csv/
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.

This comment has been minimized.

Copy link
@zekth

zekth May 23, 2019

Author Contributor

Will surely do.

@zekth zekth referenced this pull request May 23, 2019

Merged

[TOML] Move to encoding #435

@zekth zekth force-pushed the zekth:csv branch from 3846973 to a2a1e35 May 24, 2019

review
refactor

format

@zekth zekth force-pushed the zekth:csv branch from c576305 to 55022ae May 24, 2019

@zekth

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2019

Remains weird cases with double quotes and multiline in fields to handle. For me it's landable now and tests are already here. So if you want to land it now i'm OK. There is a TODO comment added for remaining cases

Show resolved Hide resolved encoding/csv.ts Outdated
Show resolved Hide resolved encoding/csv.ts Outdated

@zekth zekth changed the title [WIP] Addition of CSV Addition of CSV May 24, 2019

@ry

ry approved these changes May 24, 2019

Copy link
Contributor

left a comment

Cool! LGTM

@ry ry merged commit c8a7dcd into denoland:master May 24, 2019

5 checks passed

denoland.deno_std Build #20190524.12 succeeded
Details
denoland.deno_std (Linux) Linux succeeded
Details
denoland.deno_std (Mac) Mac succeeded
Details
denoland.deno_std (Windows) Windows succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@zekth zekth deleted the zekth:csv branch May 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.