Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(868)

Issue 548198: Rewrite CsvParser.parseLine to make it simpler and gain some performance improvement. (Closed)

Created:
10 years, 11 months ago by mnaganov (inactive)
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Rewrite CsvParser.parseLine to make it simpler and gain some performance improvement. Committed: http://code.google.com/p/v8/source/detail?r=3746

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -37 lines) Patch
M test/mjsunit/tools/csvparser.js View 1 chunk +4 lines, -0 lines 0 comments Download
M tools/csvparser.js View 2 chunks +22 lines, -37 lines 2 comments Download

Messages

Total messages: 3 (0 generated)
mnaganov (inactive)
10 years, 11 months ago (2010-01-29 08:34:31 UTC) #1
Søren Thygesen Gjesse
LGTM http://codereview.chromium.org/548198/diff/1/3 File tools/csvparser.js (right): http://codereview.chromium.org/548198/diff/1/3#newcode72 tools/csvparser.js:72: fields.push(field.replace(doubleQuoteRe, '"')); Is this covered by any tests?
10 years, 10 months ago (2010-01-29 13:21:34 UTC) #2
mnaganov (inactive)
10 years, 10 months ago (2010-01-29 13:31:10 UTC) #3
http://codereview.chromium.org/548198/diff/1/3
File tools/csvparser.js (right):

http://codereview.chromium.org/548198/diff/1/3#newcode72
tools/csvparser.js:72: fields.push(field.replace(doubleQuoteRe, '"'));
On 2010/01/29 13:21:34, Søren Gjesse wrote:
> Is this covered by any tests?

You mean double quotes? Lines 53--63 in test/mjsunit/tools/csvparser.js

Powered by Google App Engine
This is Rietveld 408576698