Skip to content

Fix #2574 #2657

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+44 −40
Open

Fix #2574 #2657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6ce6bb
some test cleanup
jonsequitur Aug 8, 2025
f3d3716
fix #2574
jonsequitur Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 32 additions & 40 deletions src/System.CommandLine.Tests/Binding/TypeConversionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,31 @@ protected T GetValue<T>(Argument<T> argument, string commandLine)
return result.GetValue(argument);
}

[Fact]
public void Option_argument_of_DirectoryInfo_can_be_bound_without_custom_conversion_logic()
{
var option = new Option<DirectoryInfo>("--dir");

var dir = new DirectoryInfo(Path.Combine(Directory.GetCurrentDirectory(), "temp"));

var result = new RootCommand { option }.Parse($"--dir {dir.FullName}");

result.GetValue(option)
.FullName
.Should()
.Be(dir.FullName);
}

[Fact]
public void Option_argument_of_FileInfo_can_be_bound_without_custom_conversion_logic()
{
var option = new Option<FileInfo>("--file");

var file = new FileInfo(Path.Combine(new DirectoryInfo("temp").FullName, "the-file.txt"));

GetValue(option, $"--file {file.FullName}")
var result = new RootCommand { option }.Parse($"--file {file.FullName}");

result.GetValue(option)
.Name
.Should()
.Be("the-file.txt");
Expand Down Expand Up @@ -107,6 +124,20 @@ public void Argument_of_array_of_FileInfo_can_be_called_without_custom_conversio
.BeEquivalentTo("file1.txt", "file2.txt");
}

[Fact] // https://github.com/dotnet/command-line-api/issues/2574
public void Option_argument_of_Uri_can_be_bound_without_custom_conversion_logic()
{
var option = new Option<Uri>("--file");

var uri = new Uri("https://example.com");

var result = new RootCommand { option }.Parse($"--file {uri}");

result.GetValue(option)
.Should()
.Be(uri);
}

[Fact]
public void Argument_defaults_arity_to_One_for_non_IEnumerable_types()
{
Expand Down Expand Up @@ -419,44 +450,6 @@ public void A_default_value_of_a_non_string_type_can_be_specified()
.Be(123);
}

[Fact]
public void A_default_value_with_a_custom_constructor_can_be_specified_for_an_option_argument()
{
var directoryInfo = new DirectoryInfo(Directory.GetCurrentDirectory());

var option = new Option<DirectoryInfo>("-x") { DefaultValueFactory = (_) => directoryInfo };

var command = new Command("something")
{
option
};

var result = command.Parse("something");

result.GetValue(option).Should().Be(directoryInfo);
}

[Fact]
public void A_default_value_with_a_custom_constructor_can_be_specified_for_a_command_argument()
{
var directoryInfo = new DirectoryInfo(Directory.GetCurrentDirectory());

var argument = new Argument<DirectoryInfo>("the-arg") { DefaultValueFactory = (_) => directoryInfo };

var command = new Command("something")
{
argument
};

var result = command.Parse("something");

result.Errors.Should().BeEmpty();

var value = result.GetValue(argument);

value.Should().Be(directoryInfo);
}

[Fact]
public void Specifying_an_option_argument_overrides_the_default_value()
{
Expand All @@ -474,7 +467,6 @@ public void Specifying_an_option_argument_overrides_the_default_value()
value.Should().Be(456);
}


[Fact]
public void Values_can_be_correctly_converted_to_DateTime_without_the_parser_specifying_a_custom_converter()
{
Expand Down
12 changes: 12 additions & 0 deletions src/System.CommandLine/Binding/ArgumentConverter.StringConverters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,5 +286,17 @@ private static Dictionary<Type, TryConvertString> StringConverters
value = default;
return false;
},

[typeof(Uri)] = (string input, out object? value) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this on purpose in #2127 in order to reduce the number of dependencies and reduce the size of a published NativeAOT app. IIRC it allows us to not include System.Private.Uri.dll for every published app.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should hold off on bringing this back because of the tradeoff. Echoing an offline discussion, I recommend that we lean into IParsable<T> and ISpanParsable<T> and even look ahead to extension interfaces as a better long-term approach for how we can handle more types without taking more hard dependencies like this in the short term.

{
if (Uri.TryCreate(input, UriKind.Absolute, out var uri))
{
value = uri;
return true;
}

value = default;
return false;
},
};
}