Skip to content

Commit a8040c5

Browse files
committed
Better error messages and fix double quote problem
- Updated to latest daily build `2.0.0-beta4.24123.1` of System.CommandLine. - Better error message when a passed class does not have a `[CliCommand]` attribute ot it's a nested class and one of its parents does not have a `[CliCommand]` attribute. - On Windows platform, backslash + double quote (`\"`) at the end of an argument, is usually a path separator and not an escape for double quote, so it will be trimmed to prevent unnecessary path errors. Related to dotnet/command-line-api#2334, dotnet/command-line-api#2276, dotnet/command-line-api#354
1 parent 890d624 commit a8040c5

File tree

210 files changed

+3804
-2647
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

210 files changed

+3804
-2647
lines changed

docs/README.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -902,10 +902,11 @@ Options:
902902
-v, --version Show version information
903903
-?, -h, --help Show help and usage information
904904
```
905-
First line comes from `AssemblyProductAttribute` or `AssemblyName`.
906-
Version comes from `AssemblyInformationalVersionAttribute` or `AssemblyFileVersionAttribute` or `AssemblyVersionAttribute`.
907-
Second line comes from `AssemblyCopyrightAttribute`.
908-
Third line comes from `CliCommand.Description` or `AssemblyDescriptionAttribute`.
905+
- First line comes from `AssemblyProductAttribute` or `AssemblyName` (`<Product>` tag in your .csproj file).
906+
Version comes from `AssemblyInformationalVersionAttribute` or `AssemblyFileVersionAttribute` or `AssemblyVersionAttribute`
907+
(`<InformationalVersion>` or `<FileVersion >` or `<Version>` tag in your .csproj file).
908+
- Second line comes from `AssemblyCopyrightAttribute` (`<Copyright>` tag in your .csproj file).
909+
- Third line comes from `Description` property of `[CliCommand]` or for root commands, `AssemblyDescriptionAttribute` (`<Description>` tag in your .csproj file).
909910

910911
Note, how command/option/argument names, descriptions and default values are automatically populated.
911912

src/Directory.Build.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
<PropertyGroup>
44
<!-- https://learn.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#generateassemblyinfo -->
5-
<VersionPrefix>1.8.1</VersionPrefix>
5+
<VersionPrefix>1.8.2</VersionPrefix>
66
<Product>DotMake Command-Line</Product>
77
<Company>DotMake</Company>
88
<!-- Copyright is also used for NuGet metadata -->

src/DotMake.CommandLine.SourceGeneration/CliArgumentInfo.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public CliArgumentInfo(ISymbol symbol, SyntaxNode syntaxNode, AttributeData attr
2626
Symbol = (IPropertySymbol)symbol;
2727
Parent = parent;
2828

29-
ParseInfo = new CliArgumentParseInfo(Symbol, syntaxNode, semanticModel, this);
29+
ParseInfo = new CliArgumentParserInfo(Symbol, syntaxNode, semanticModel, this);
3030

3131
Analyze();
3232

@@ -61,7 +61,7 @@ public CliArgumentInfo(GeneratorAttributeSyntaxContext attributeSyntaxContext)
6161

6262
public bool Required { get; }
6363

64-
public CliArgumentParseInfo ParseInfo { get; set; }
64+
public CliArgumentParserInfo ParseInfo { get; set; }
6565

6666
private void Analyze()
6767
{
@@ -124,9 +124,12 @@ public void AppendCSharpCreateString(CodeStringBuilder sb, string varName, strin
124124
break;
125125
}
126126
}
127-
}
128127

129-
ParseInfo.AppendCSharpCallString(sb, $"{varName}.CustomParser");
128+
if (!Required)
129+
sb.AppendLine($"DefaultValueFactory = _ => {varDefaultValue},");
130+
131+
ParseInfo.AppendCSharpCallString(sb, "CustomParser", ",");
132+
}
130133

131134
if (AttributeArguments.TryGetTypedConstant(nameof(CliArgumentAttribute.AllowedValues), out var allowedValuesTypedConstant))
132135
sb.AppendLine($"{varName}.AcceptOnlyFromAmong(new[] {allowedValuesTypedConstant.ToCSharpString()});");
@@ -142,9 +145,6 @@ public void AppendCSharpCreateString(CodeStringBuilder sb, string varName, strin
142145
sb.AppendLine($"DotMake.CommandLine.CliValidationExtensions.AddValidator({varName}, {validationPatternTypedConstant.ToCSharpString()});");
143146
}
144147

145-
if (!Required)
146-
sb.AppendLine($"{varName}.DefaultValueFactory = _ => {varDefaultValue};");
147-
148148
//In ArgumentArity.Default, Arity is set to ZeroOrMore for IEnumerable if parent is command,
149149
//but we want to enforce OneOrMore so that Required is consistent
150150
if (Required

src/DotMake.CommandLine.SourceGeneration/CliArgumentParseInfo.cs renamed to src/DotMake.CommandLine.SourceGeneration/CliArgumentParserInfo.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
namespace DotMake.CommandLine.SourceGeneration
77
{
8-
public class CliArgumentParseInfo : CliSymbolInfo, IEquatable<CliArgumentParseInfo>
8+
public class CliArgumentParserInfo : CliSymbolInfo, IEquatable<CliArgumentParserInfo>
99
{
1010
private static readonly HashSet<string> SupportedConverters = new HashSet<string>
1111
{
@@ -43,7 +43,7 @@ public class CliArgumentParseInfo : CliSymbolInfo, IEquatable<CliArgumentParseIn
4343
"System.Net.IPEndPoint"
4444
};
4545

46-
public CliArgumentParseInfo(IPropertySymbol symbol, SyntaxNode syntaxNode, SemanticModel semanticModel, CliSymbolInfo parent)
46+
public CliArgumentParserInfo(IPropertySymbol symbol, SyntaxNode syntaxNode, SemanticModel semanticModel, CliSymbolInfo parent)
4747
: base(symbol, syntaxNode, semanticModel)
4848
{
4949
Symbol = symbol;
@@ -109,11 +109,11 @@ private void Analyze()
109109
}
110110
}
111111

112-
public void AppendCSharpCallString(CodeStringBuilder sb, string varCustomParser)
112+
public void AppendCSharpCallString(CodeStringBuilder sb, string varCustomParser, string afterBlock)
113113
{
114114
if (ItemType != null)
115115
{
116-
using (sb.AppendParamsBlockStart($"{varCustomParser} = GetParseArgument<{Type.ToReferenceString()}, {ItemType.ToReferenceString()}>", ";"))
116+
using (sb.AppendParamsBlockStart($"{varCustomParser} = GetArgumentParser<{Type.ToReferenceString()}, {ItemType.ToReferenceString()}>", afterBlock))
117117
{
118118
if (Converter == null)
119119
sb.AppendLine("null,");
@@ -141,7 +141,7 @@ public void AppendCSharpCallString(CodeStringBuilder sb, string varCustomParser)
141141
{
142142
//Even if argument type does not need a converter, use a ParseArgument method,
143143
//so that our custom converter is used for supporting all collection compatible types.
144-
using (sb.AppendParamsBlockStart($"{varCustomParser} = GetParseArgument<{Type.ToReferenceString()}>", ";"))
144+
using (sb.AppendParamsBlockStart($"{varCustomParser} = GetArgumentParser<{Type.ToReferenceString()}>", afterBlock))
145145
{
146146
if (Converter == null)
147147
sb.AppendLine("null");
@@ -153,7 +153,7 @@ public void AppendCSharpCallString(CodeStringBuilder sb, string varCustomParser)
153153
}
154154
}
155155

156-
public bool Equals(CliArgumentParseInfo other)
156+
public bool Equals(CliArgumentParserInfo other)
157157
{
158158
return base.Equals(other);
159159
}

src/DotMake.CommandLine.SourceGeneration/CliCommandAsDelegateInfo.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace DotMake.CommandLine.SourceGeneration
1111
public class CliCommandAsDelegateInfo : CliSymbolInfo, IEquatable<CliCommandAsDelegateInfo>
1212
{
1313
private const string TaskFullName = "System.Threading.Tasks.Task";
14-
public static readonly string CliCommandAsDelegateDefinitionFullName = "DotMake.CommandLine.CliCommandAsDelegateDefinition";
14+
public static readonly string CliCommandAsDelegateFullName = "DotMake.CommandLine.CliCommandAsDelegate";
1515

1616
public CliCommandAsDelegateInfo(ISymbol symbol, SyntaxNode syntaxNode, SemanticModel semanticModel)
1717
: base(symbol, syntaxNode, semanticModel)
@@ -109,7 +109,7 @@ public string GenerateHash()
109109
return GenerateString().GetStableStringHashCode32();
110110
}
111111

112-
//The generated string should match the one generated in DotMake.CommandLine.CliCommandAsDelegateDefinition
112+
//The generated string should match the one generated in DotMake.CommandLine.CliCommandAsDelegate
113113
//So that the generated hash matches.
114114
public string GenerateString()
115115
{
@@ -199,7 +199,7 @@ public void AppendCSharpDefineString(CodeStringBuilder sb)
199199
else
200200
sb.AppendLine($"[{CliCommandInfo.AttributeFullName}]");
201201

202-
using (sb.AppendBlockStart($"public class {GeneratedClassName} : {CliCommandAsDelegateDefinitionFullName}"))
202+
using (sb.AppendBlockStart($"public class {GeneratedClassName} : {CliCommandAsDelegateFullName}"))
203203
{
204204
for (var index = 0; index < ParameterInfos.Count; index++)
205205
{

src/DotMake.CommandLine.SourceGeneration/CliOptionInfo.cs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public CliOptionInfo(ISymbol symbol, SyntaxNode syntaxNode, AttributeData attrib
2626
Symbol = (IPropertySymbol)symbol;
2727
Parent = parent;
2828

29-
ParseInfo = new CliArgumentParseInfo(Symbol, syntaxNode, semanticModel, this);
29+
ParseInfo = new CliArgumentParserInfo(Symbol, syntaxNode, semanticModel, this);
3030

3131
Analyze();
3232

@@ -62,7 +62,7 @@ public CliOptionInfo(GeneratorAttributeSyntaxContext attributeSyntaxContext)
6262

6363
public bool Required { get; }
6464

65-
public CliArgumentParseInfo ParseInfo { get; set; }
65+
public CliArgumentParserInfo ParseInfo { get; set; }
6666

6767
private void Analyze()
6868
{
@@ -132,9 +132,11 @@ public void AppendCSharpCreateString(CodeStringBuilder sb, string varName, strin
132132

133133
//Required is special as it can be calculated when CliOptionAttribute.Required is missing (not forced)
134134
sb.AppendLine($"Required = {Required.ToString().ToLowerInvariant()},");
135-
}
135+
if (!Required)
136+
sb.AppendLine($"DefaultValueFactory = _ => {varDefaultValue},");
136137

137-
ParseInfo.AppendCSharpCallString(sb, $"{varName}.CustomParser");
138+
ParseInfo.AppendCSharpCallString(sb, "CustomParser", ",");
139+
}
138140

139141
if (AttributeArguments.TryGetTypedConstant(nameof(CliOptionAttribute.AllowedValues), out var allowedValuesTypedConstant))
140142
sb.AppendLine($"{varName}.AcceptOnlyFromAmong(new[] {allowedValuesTypedConstant.ToCSharpString()});");
@@ -150,9 +152,6 @@ public void AppendCSharpCreateString(CodeStringBuilder sb, string varName, strin
150152
sb.AppendLine($"DotMake.CommandLine.CliValidationExtensions.AddValidator({varName}, {validationPatternTypedConstant.ToCSharpString()});");
151153
}
152154

153-
if (!Required)
154-
sb.AppendLine($"{varName}.DefaultValueFactory = _ => {varDefaultValue};");
155-
156155
var shortForm = optionName.RemovePrefix();
157156
if (Parent.Settings.ShortFormAutoGenerate && shortForm.Length >= 2)
158157
{

src/DotMake.CommandLine.SourceGeneration/DotMake.CommandLine.SourceGeneration.csproj

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,4 @@
5353
<EmbeddedResource Remove="..\DotMake.CommandLine.SourceGeneration.Embedded\obj\**\*" />
5454
</ItemGroup>
5555

56-
<ItemGroup>
57-
<EmbeddedResource Remove="..\DotMake.CommandLine.SourceGeneration.Embedded\CliServiceExtensions.cs" />
58-
</ItemGroup>
59-
6056
</Project>

src/DotMake.CommandLine/Cli.cs

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.CommandLine.Help;
55
using System.CommandLine.Invocation;
66
using System.Linq;
7+
using System.Runtime.InteropServices;
78
using System.Threading;
89
using System.Threading.Tasks;
910

@@ -41,16 +42,20 @@ public static class Cli
4142

4243
/// <summary>
4344
/// Returns a string array containing the command-line arguments for the current process.
45+
/// <para>
4446
/// Uses <see cref="Environment.GetCommandLineArgs"/> but skips the first element which is the executable file name,
4547
/// so the following zero or more elements that contain the remaining command-line arguments are returned,
4648
/// i.e. returns the same as the special variable <c>args</c> available in <c>Program.cs</c> (new style with top-level statements)
4749
/// or as the string array passed to the program's <c>Main</c> method (old style).
50+
/// </para>
51+
/// <para>Also on Windows platform, backslash + double quote (<c>\&quot;</c>) at the end of an argument,
52+
/// is usually a path separator and not an escape for double quote, so it will be trimmed to prevent unnecessary path errors.</para>
4853
/// </summary>
4954
/// <returns>An array of strings where each element contains a command-line argument.</returns>
5055
public static string[] GetArgs()
5156
{
5257
if (Environment.GetCommandLineArgs() is { Length: > 0 } args)
53-
return args.Skip(1).ToArray();
58+
return FixArgs(args.Skip(1).ToArray());
5459

5560
return Array.Empty<string>();
5661
}
@@ -156,7 +161,7 @@ public static int Run<TDefinition>(string[] args = null, CliSettings settings =
156161
{
157162
var configuration = GetConfiguration<TDefinition>(settings);
158163

159-
return configuration.Invoke(args ?? GetArgs());
164+
return configuration.Invoke(FixArgs(args) ?? GetArgs());
160165
}
161166

162167
/// <summary>
@@ -200,7 +205,7 @@ public static int Run<TDefinition>(string commandLine, CliSettings settings = nu
200205
/// </example>
201206
public static int Run(Delegate cliCommandAsDelegate, CliSettings settings = null)
202207
{
203-
var definitionType = CliCommandAsDelegateDefinition.Get(cliCommandAsDelegate);
208+
var definitionType = CliCommandAsDelegate.Get(cliCommandAsDelegate);
204209
var configuration = GetConfiguration(definitionType, settings);
205210

206211
return configuration.Invoke(GetArgs());
@@ -224,7 +229,7 @@ public static async Task<int> RunAsync<TDefinition>(string[] args = null, CliSet
224229
{
225230
var configuration = GetConfiguration<TDefinition>(settings);
226231

227-
return await configuration.InvokeAsync(args ?? GetArgs(), cancellationToken);
232+
return await configuration.InvokeAsync(FixArgs(args) ?? GetArgs(), cancellationToken);
228233
}
229234

230235
/// <summary>
@@ -259,7 +264,7 @@ public static async Task<int> RunAsync<TDefinition>(string commandLine, CliSetti
259264
/// </example>
260265
public static async Task<int> RunAsync(Delegate cliCommandAsDelegate, CliSettings settings = null, CancellationToken cancellationToken = default)
261266
{
262-
var definitionType = CliCommandAsDelegateDefinition.Get(cliCommandAsDelegate);
267+
var definitionType = CliCommandAsDelegate.Get(cliCommandAsDelegate);
263268
var configuration = GetConfiguration(definitionType, settings);
264269

265270
return await configuration.InvokeAsync(GetArgs(), cancellationToken);
@@ -281,7 +286,7 @@ public static ParseResult Parse<TDefinition>(string[] args = null, CliSettings s
281286
{
282287
var configuration = GetConfiguration<TDefinition>(settings);
283288

284-
return configuration.Parse(args ?? GetArgs());
289+
return configuration.Parse(FixArgs(args) ?? GetArgs());
285290
}
286291

287292
/// <summary>
@@ -316,6 +321,36 @@ public static TDefinition Bind<TDefinition>(this ParseResult parseResult)
316321
return (TDefinition)commandBuilder.Bind(parseResult);
317322
}
318323

324+
private static string[] FixArgs(string[] args)
325+
{
326+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
327+
&& args != null)
328+
{
329+
/*
330+
On Windows, trim ending double quote:
331+
For example, if a path parameter is passed like this:
332+
--source "C:\myfiles\"
333+
The value comes as
334+
C:\myfiles"
335+
due to CommandLineToArgvW reading backslash as an escape for double quote character.
336+
As on Windows, backslash at the end is usually a path separator, trim it to prevent unnecessary errors.
337+
Note that this is not required for commandLine string as in that case SplitCommandLine is used,
338+
and it already trims double quote characters
339+
340+
https://devblogs.microsoft.com/oldnewthing/20100917-00/?p=12833
341+
https://github.com/dotnet/command-line-api/issues/2334
342+
https://github.com/dotnet/command-line-api/issues/2276
343+
https://github.com/dotnet/command-line-api/issues/354
344+
*/
345+
for (var index = 0; index < args.Length; index++)
346+
{
347+
args[index] = args[index].TrimEnd('"');
348+
}
349+
}
350+
351+
return args;
352+
}
353+
319354
private sealed class VersionOptionAction : SynchronousCliAction
320355
{
321356
public override int Invoke(ParseResult parseResult)

src/DotMake.CommandLine/CliCommandAsDelegateDefinition.cs renamed to src/DotMake.CommandLine/CliCommandAsDelegate.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace DotMake.CommandLine
99
/// <summary>
1010
/// Represents a definition class generated by the source generator for a command as delegate.
1111
/// </summary>
12-
public class CliCommandAsDelegateDefinition
12+
public class CliCommandAsDelegate
1313
{
1414
/// <summary>
1515
/// Invokes the method represented by the command as delegate.
@@ -25,7 +25,7 @@ public object InvokeDelegate(string hash, object[] args)
2525
{
2626
if (!RegisteredDelegates.TryGetValue(hash, out var delegateInstance))
2727
throw new Exception($"A registered command as delegate instance is not found for hash '{hash}'. " +
28-
$"Please ensure CliCommandAsDelegateDefinition.Get() is called first.");
28+
$"Please ensure CliCommandAsDelegate.Get() is called first.");
2929

3030
return delegateInstance.DynamicInvoke(args);
3131
}

0 commit comments

Comments
 (0)