Skip to content

Commit 47618e0

Browse files
rmunnrmunn
committed
Fix enumerable options grabbing too many values
Fixes #687 Fixes #619 Fixes #617 Fixes #510 Fixes #454 Fixes #420 Fixes #396 Fixes #91
1 parent 570d7b7 commit 47618e0

File tree

7 files changed

+339
-30
lines changed

7 files changed

+339
-30
lines changed

‎src/CommandLine/Core/GetoptTokenizer.cs

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,28 @@ public static Result<IEnumerable<Token>, Error> ExplodeOptionList(
9494
{
9595
var tokens = tokenizerResult.SucceededWith().Memoize();
9696

97-
var replaces = tokens.Select((t, i) =>
98-
optionSequenceWithSeparatorLookup(t.Text)
99-
.MapValueOrDefault(sep => Tuple.Create(i + 1, sep),
100-
Tuple.Create(-1, '\0'))).SkipWhile(x => x.Item1 < 0).Memoize();
101-
102-
var exploded = tokens.Select((t, i) =>
103-
replaces.FirstOrDefault(x => x.Item1 == i).ToMaybe()
104-
.MapValueOrDefault(r => t.Text.Split(r.Item2).Select(Token.Value),
105-
Enumerable.Empty<Token>().Concat(new[] { t })));
106-
107-
var flattened = exploded.SelectMany(x => x);
108-
109-
return Result.Succeed(flattened, tokenizerResult.SuccessMessages());
97+
var exploded = new List<Token>(tokens is ICollection<Token> coll ? coll.Count : tokens.Count());
98+
var nothing = Maybe.Nothing<char>(); // Re-use same Nothing instance for efficiency
99+
var separator = nothing;
100+
foreach (var token in tokens) {
101+
if (token.IsName()) {
102+
separator = optionSequenceWithSeparatorLookup(token.Text);
103+
exploded.Add(token);
104+
} else {
105+
// Forced values are never considered option values, so they should not be split
106+
if (separator.MatchJust(out char sep) && sep != '\0' && !token.IsValueForced()) {
107+
if (token.Text.Contains(sep)) {
108+
exploded.AddRange(token.Text.Split(sep).Select(Token.ValueFromSeparator));
109+
} else {
110+
exploded.Add(token);
111+
}
112+
} else {
113+
exploded.Add(token);
114+
}
115+
separator = nothing; // Only first value after a separator can possibly be split
116+
}
117+
}
118+
return Result.Succeed(exploded as IEnumerable<Token>, tokenizerResult.SuccessMessages());
110119
}
111120

112121
public static Func<

‎src/CommandLine/Core/Token.cs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,12 @@ public static Token Value(string text, bool explicitlyAssigned)
3434

3535
public static Token ValueForced(string text)
3636
{
37-
return new Value(text, false, true);
37+
return new Value(text, false, true, false);
38+
}
39+
40+
public static Token ValueFromSeparator(string text)
41+
{
42+
return new Value(text, false, false, true);
3843
}
3944

4045
public TokenType Tag
@@ -86,29 +91,45 @@ class Value : Token, IEquatable<Value>
8691
{
8792
private readonly bool explicitlyAssigned;
8893
private readonly bool forced;
94+
private readonly bool fromSeparator;
8995

9096
public Value(string text)
91-
: this(text, false, false)
97+
: this(text, false, false, false)
9298
{
9399
}
94100

95101
public Value(string text, bool explicitlyAssigned)
96-
: this(text, explicitlyAssigned, false)
102+
: this(text, explicitlyAssigned, false, false)
97103
{
98104
}
99105

100-
public Value(string text, bool explicitlyAssigned, bool forced)
106+
public Value(string text, bool explicitlyAssigned, bool forced, bool fromSeparator)
101107
: base(TokenType.Value, text)
102108
{
103109
this.explicitlyAssigned = explicitlyAssigned;
104110
this.forced = forced;
111+
this.fromSeparator = fromSeparator;
105112
}
106113

114+
/// <summary>
115+
/// Whether this value came from a long option with "=" separating the name from the value
116+
/// </summary>
107117
public bool ExplicitlyAssigned
108118
{
109119
get { return explicitlyAssigned; }
110120
}
111121

122+
/// <summary>
123+
/// Whether this value came from a sequence specified with a separator (e.g., "--files a.txt,b.txt,c.txt")
124+
/// </summary>
125+
public bool FromSeparator
126+
{
127+
get { return fromSeparator; }
128+
}
129+
130+
/// <summary>
131+
/// Whether this value came from args after the -- separator (when EnableDashDash = true)
132+
/// </summary>
112133
public bool Forced
113134
{
114135
get { return forced; }
@@ -153,6 +174,11 @@ public static bool IsValue(this Token token)
153174
return token.Tag == TokenType.Value;
154175
}
155176

177+
public static bool IsValueFromSeparator(this Token token)
178+
{
179+
return token.IsValue() && ((Value)token).FromSeparator;
180+
}
181+
156182
public static bool IsValueForced(this Token token)
157183
{
158184
return token.IsValue() && ((Value)token).Forced;

‎src/CommandLine/Core/TokenPartitioner.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,18 @@ public static Tuple<IEnumerable<Token>, IEnumerable<Token>, IEnumerable<Token>,
4747
var count = new Dictionary<Token, int>();
4848
var max = new Dictionary<Token, Maybe<int>>();
4949
var state = SequenceState.TokenSearch;
50+
var separatorSeen = false;
5051
Token nameToken = null;
5152
foreach (var token in tokens)
5253
{
5354
if (token.IsValueForced())
5455
{
56+
separatorSeen = false;
5557
nonOptionTokens.Add(token);
5658
}
5759
else if (token.IsName())
5860
{
61+
separatorSeen = false;
5962
if (typeLookup(token.Text).MatchJust(out var info))
6063
{
6164
switch (info.TargetType)
@@ -96,12 +99,14 @@ public static Tuple<IEnumerable<Token>, IEnumerable<Token>, IEnumerable<Token>,
9699
case SequenceState.TokenSearch:
97100
case SequenceState.ScalarTokenFound when nameToken == null:
98101
case SequenceState.SequenceTokenFound when nameToken == null:
102+
separatorSeen = false;
99103
nameToken = null;
100104
nonOptionTokens.Add(token);
101105
state = SequenceState.TokenSearch;
102106
break;
103107

104108
case SequenceState.ScalarTokenFound:
109+
separatorSeen = false;
105110
nameToken = null;
106111
scalarTokens.Add(token);
107112
state = SequenceState.TokenSearch;
@@ -116,6 +121,20 @@ public static Tuple<IEnumerable<Token>, IEnumerable<Token>, IEnumerable<Token>,
116121
nonOptionTokens.Add(token);
117122
state = SequenceState.TokenSearch;
118123
}
124+
else if (token.IsValueFromSeparator())
125+
{
126+
separatorSeen = true;
127+
sequence.Add(token);
128+
count[nameToken]++;
129+
}
130+
else if (separatorSeen)
131+
{
132+
// Previous token came from a separator but this one didn't: sequence is completed
133+
separatorSeen = false;
134+
nameToken = null;
135+
nonOptionTokens.Add(token);
136+
state = SequenceState.TokenSearch;
137+
}
119138
else
120139
{
121140
sequence.Add(token);
@@ -125,6 +144,7 @@ public static Tuple<IEnumerable<Token>, IEnumerable<Token>, IEnumerable<Token>,
125144
else
126145
{
127146
// Should never get here, but just in case:
147+
separatorSeen = false;
128148
sequences[nameToken] = new List<Token>(new[] { token });
129149
count[nameToken] = 0;
130150
max[nameToken] = Maybe.Nothing<int>();

‎src/CommandLine/Core/Tokenizer.cs

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,28 @@ public static Result<IEnumerable<Token>, Error> ExplodeOptionList(
6262
{
6363
var tokens = tokenizerResult.SucceededWith().Memoize();
6464

65-
var replaces = tokens.Select((t, i) =>
66-
optionSequenceWithSeparatorLookup(t.Text)
67-
.MapValueOrDefault(sep => Tuple.Create(i + 1, sep),
68-
Tuple.Create(-1, '\0'))).SkipWhile(x => x.Item1 < 0).Memoize();
69-
70-
var exploded = tokens.Select((t, i) =>
71-
replaces.FirstOrDefault(x => x.Item1 == i).ToMaybe()
72-
.MapValueOrDefault(r => t.Text.Split(r.Item2).Select(Token.Value),
73-
Enumerable.Empty<Token>().Concat(new[] { t })));
74-
75-
var flattened = exploded.SelectMany(x => x);
76-
77-
return Result.Succeed(flattened, tokenizerResult.SuccessMessages());
65+
var exploded = new List<Token>(tokens is ICollection<Token> coll ? coll.Count : tokens.Count());
66+
var nothing = Maybe.Nothing<char>(); // Re-use same Nothing instance for efficiency
67+
var separator = nothing;
68+
foreach (var token in tokens) {
69+
if (token.IsName()) {
70+
separator = optionSequenceWithSeparatorLookup(token.Text);
71+
exploded.Add(token);
72+
} else {
73+
// Forced values are never considered option values, so they should not be split
74+
if (separator.MatchJust(out char sep) && sep != '\0' && !token.IsValueForced()) {
75+
if (token.Text.Contains(sep)) {
76+
exploded.AddRange(token.Text.Split(sep).Select(Token.ValueFromSeparator));
77+
} else {
78+
exploded.Add(token);
79+
}
80+
} else {
81+
exploded.Add(token);
82+
}
83+
separator = nothing; // Only first value after a separator can possibly be split
84+
}
85+
}
86+
return Result.Succeed(exploded as IEnumerable<Token>, tokenizerResult.SuccessMessages());
7887
}
7988

8089
public static IEnumerable<Token> Normalize(

‎tests/CommandLine.Tests/Fakes/Options_With_Sequence_Having_Separator_And_Values.cs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information.
2+
3+
using System.Collections.Generic;
4+
5+
namespace CommandLine.Tests.Fakes
6+
{
7+
public class Options_For_Issue_91
8+
{
9+
[Value(0, Required = true)]
10+
public string InputFileName { get; set; }
11+
12+
[Option('o', "output")]
13+
public string OutputFileName { get; set; }
14+
15+
[Option('i', "include", Separator = ',')]
16+
public IEnumerable<string> Included { get; set; }
17+
18+
[Option('e', "exclude", Separator = ',')]
19+
public IEnumerable<string> Excluded { get; set; }
20+
}
21+
22+
public class Options_For_Issue_454
23+
{
24+
[Option('c', "channels", Required = true, Separator = ':', HelpText = "Channel names")]
25+
public IEnumerable<string> Channels { get; set; }
26+
27+
[Value(0, Required = true, MetaName = "file_path", HelpText = "Path of archive to be processed")]
28+
public string ArchivePath { get; set; }
29+
}
30+
31+
public class Options_For_Issue_510
32+
{
33+
[Option('a', "aa", Required = false, Separator = ',')]
34+
public IEnumerable<string> A { get; set; }
35+
36+
[Option('b', "bb", Required = false)]
37+
public string B { get; set; }
38+
39+
[Value(0, Required = true)]
40+
public string C { get; set; }
41+
}
42+
43+
public enum FMode { C, D, S };
44+
45+
public class Options_For_Issue_617
46+
{
47+
[Option("fm", Separator=',', Default = new[] { FMode.S })]
48+
public IEnumerable<FMode> Mode { get; set; }
49+
50+
[Option('q')]
51+
public bool q { get;set; }
52+
53+
[Value(0)]
54+
public IList<string> Files { get; set; }
55+
}
56+
57+
public class Options_For_Issue_619
58+
{
59+
[Option("verbose", Required = false, Default = false, HelpText = "Generate process tracing information")]
60+
public bool Verbose { get; set; }
61+
62+
[Option("outdir", Required = false, Default = ".", HelpText = "Directory to look for object file")]
63+
public string OutDir { get; set; }
64+
65+
[Option("modules", Required = true, Separator = ',', HelpText = "Directories to look for module file")]
66+
public IEnumerable<string> ModuleDirs { get; set; }
67+
68+
[Option("ignore", Required = false, Separator = ' ', HelpText = "List of additional module name references to ignore")]
69+
public IEnumerable<string> Ignores { get; set; }
70+
71+
[Value(0, Required = true, HelpText = "List of source files to process")]
72+
public IEnumerable<string> Srcs { get; set; }
73+
}
74+
}

‎tests/CommandLine.Tests/Fakes/Options_With_Similar_Names.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information.
2+
3+
using System.Collections.Generic;
4+
5+
namespace CommandLine.Tests.Fakes
6+
{
7+
public class Options_With_Similar_Names
8+
{
9+
[Option("deploy", Separator = ',', HelpText= "Projects to deploy")]
10+
public IEnumerable<string> Deploys { get; set; }
11+
12+
[Option("profile", Required = true, HelpText = "Profile to use when restoring and publishing")]
13+
public string Profile { get; set; }
14+
15+
[Option("configure-profile", Required = true, HelpText = "Profile to use for Configure")]
16+
public string ConfigureProfile { get; set; }
17+
}
18+
19+
public class Options_With_Similar_Names_And_Separator
20+
{
21+
[Option('f', "flag", HelpText = "Flag")]
22+
public bool Flag { get; set; }
23+
24+
[Option('c', "categories", Required = false, Separator = ',', HelpText = "Categories")]
25+
public IEnumerable<string> Categories { get; set; }
26+
27+
[Option('j', "jobId", Required = true, HelpText = "Texts.ExplainJob")]
28+
public int JobId { get; set; }
29+
}
30+
31+
}

0 commit comments

Comments
 (0)