Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2683)

Issue 975463004: Parse comma-separated multiple values. (Closed)

Created:
5 years, 5 months ago by nweiz
Modified:
5 years, 5 months ago
Reviewers:
Bob Nystrom
CC:
reviews_dartlang.org
Base URL:
git@github.com:dart-lang/args@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Fix allowed. #

Total comments: 11

Patch Set 3 : Code review changes #

Total comments: 1

Patch Set 4 : Add a splitCommas option. #

Total comments: 8

Patch Set 5 : Code review changes #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+139 lines, -23 lines) Patch
M CHANGELOG.md View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M README.md View 1 2 3 4 1 chunk +11 lines, -0 lines 0 comments Download
M lib/src/arg_parser.dart View 1 2 3 4 2 chunks +11 lines, -4 lines 2 comments Download
M lib/src/option.dart View 1 2 3 4 3 chunks +13 lines, -4 lines 0 comments Download
M lib/src/parser.dart View 1 2 3 5 chunks +37 lines, -14 lines 0 comments Download
M pubspec.yaml View 1 chunk +1 line, -1 line 0 comments Download
M test/args_test.dart View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M test/parse_test.dart View 1 2 3 2 chunks +53 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
nweiz
5 years, 5 months ago (2015-03-03 01:17:51 UTC) #1
Bob Nystrom
https://codereview.chromium.org/975463004/diff/20001/lib/src/parser.dart File lib/src/parser.dart (right): https://codereview.chromium.org/975463004/diff/20001/lib/src/parser.dart#newcode264 lib/src/parser.dart:264: assert(value is String); Value can only be a string ...
5 years, 5 months ago (2015-03-03 17:36:26 UTC) #2
Sean Eagan
On 2015/03/03 01:17:51, nweiz wrote: What is the workaround if the individual values can contain ...
5 years, 5 months ago (2015-03-03 20:17:47 UTC) #3
Sean Eagan
https://codereview.chromium.org/975463004/diff/20001/lib/src/parser.dart File lib/src/parser.dart (right): https://codereview.chromium.org/975463004/diff/20001/lib/src/parser.dart#newcode264 lib/src/parser.dart:264: assert(value is String); On 2015/03/03 17:36:26, Bob Nystrom wrote: ...
5 years, 5 months ago (2015-03-03 20:21:23 UTC) #4
nweiz
Code review changes
5 years, 5 months ago (2015-03-03 20:21:48 UTC) #5
nweiz
https://codereview.chromium.org/975463004/diff/20001/lib/src/parser.dart File lib/src/parser.dart (right): https://codereview.chromium.org/975463004/diff/20001/lib/src/parser.dart#newcode264 lib/src/parser.dart:264: assert(value is String); On 2015/03/03 17:36:26, Bob Nystrom wrote: ...
5 years, 5 months ago (2015-03-03 20:22:18 UTC) #6
nweiz
On 2015/03/03 20:17:47, Sean Eagan wrote: > On 2015/03/03 01:17:51, nweiz wrote: > > What ...
5 years, 5 months ago (2015-03-03 20:39:40 UTC) #7
Bob Nystrom
On 2015/03/03 20:39:40, nweiz wrote: > On 2015/03/03 20:17:47, Sean Eagan wrote: > > On ...
5 years, 5 months ago (2015-03-03 21:19:16 UTC) #8
nweiz
Even if we add the flag, I think commas should be parsed by default. Wanting ...
5 years, 5 months ago (2015-03-03 21:54:48 UTC) #9
Bob Nystrom
On 2015/03/03 21:54:48, nweiz wrote: > Even if we add the flag, I think commas ...
5 years, 5 months ago (2015-03-03 23:42:56 UTC) #10
nweiz
On 2015/03/03 23:42:56, Bob Nystrom wrote: > On 2015/03/03 21:54:48, nweiz wrote: > > Even ...
5 years, 5 months ago (2015-03-03 23:58:05 UTC) #11
Bob Nystrom
On 2015/03/03 23:58:05, nweiz wrote: > On 2015/03/03 23:42:56, Bob Nystrom wrote: > > On ...
5 years, 5 months ago (2015-03-04 00:16:48 UTC) #12
nweiz
Added a splitCommas option; PTAL.
5 years, 5 months ago (2015-03-04 01:52:45 UTC) #13
Bob Nystrom
https://chromiumcodereview.appspot.com/975463004/diff/60001/README.md File README.md (right): https://chromiumcodereview.appspot.com/975463004/diff/60001/README.md#newcode192 README.md:192: By default, multiple options' values may also be separated ...
5 years, 5 months ago (2015-03-04 22:01:37 UTC) #14
nweiz
Code review changes
5 years, 5 months ago (2015-03-04 22:05:35 UTC) #15
nweiz
https://chromiumcodereview.appspot.com/975463004/diff/60001/README.md File README.md (right): https://chromiumcodereview.appspot.com/975463004/diff/60001/README.md#newcode192 README.md:192: By default, multiple options' values may also be separated ...
5 years, 5 months ago (2015-03-04 22:05:41 UTC) #16
Bob Nystrom
lgtm
5 years, 5 months ago (2015-03-04 22:27:31 UTC) #17
Sean Eagan
https://chromiumcodereview.appspot.com/975463004/diff/80001/lib/src/arg_parser.dart File lib/src/arg_parser.dart (right): https://chromiumcodereview.appspot.com/975463004/diff/80001/lib/src/arg_parser.dart#newcode87 lib/src/arg_parser.dart:87: 'splitCommas may not be set if allowMultiple is false.'); ...
5 years, 5 months ago (2015-03-04 22:28:03 UTC) #18
nweiz
https://chromiumcodereview.appspot.com/975463004/diff/80001/lib/src/arg_parser.dart File lib/src/arg_parser.dart (right): https://chromiumcodereview.appspot.com/975463004/diff/80001/lib/src/arg_parser.dart#newcode87 lib/src/arg_parser.dart:87: 'splitCommas may not be set if allowMultiple is false.'); ...
5 years, 5 months ago (2015-03-04 22:39:55 UTC) #19
nweiz
5 years, 5 months ago (2015-03-05 01:54:36 UTC) #20
Message was sent while issue was closed.
Committed patchset #5 (id:80001) manually as
52fe6a182296ddf705379790b41012d69d5e4ed1 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698