Skip to content
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.

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

Add ProcessStartInfo constructor that accepts IEnumerable<string> arguments #86346

Merged
merged 9 commits into from
May 19, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ public sealed partial class ProcessStartInfo
public ProcessStartInfo() { }
public ProcessStartInfo(string fileName) { }
public ProcessStartInfo(string fileName, string arguments) { }
public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable<string> arguments) { }
public System.Collections.ObjectModel.Collection<string> ArgumentList { get { throw null; } }
[System.Diagnostics.CodeAnalysis.AllowNullAttribute]
public string Arguments { get { throw null; } set { } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1330,13 +1330,7 @@ public static Process Start(string fileName, IEnumerable<string> arguments)
ArgumentNullException.ThrowIfNull(fileName);
ArgumentNullException.ThrowIfNull(arguments);

var startInfo = new ProcessStartInfo(fileName);
foreach (string argument in arguments)
{
startInfo.ArgumentList.Add(argument);
}

return Start(startInfo)!;
return Start(new ProcessStartInfo(fileName, arguments))!;
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
}

/// <devdoc>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@ public ProcessStartInfo(string fileName, string arguments)
_arguments = arguments;
}

/// <devdoc>
/// Specifies the name of the application that is to be started, as well as a set
/// of command line arguments to pass to the application.
/// </devdoc>
Copy link
Member

Choose a reason for hiding this comment

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

devdoc is relic of the past (it's not documented anywhere, you had no chance to know about it), please use standard summary tags

Suggested change
/// <devdoc>
/// Specifies the name of the application that is to be started, as well as a set
/// of command line arguments to pass to the application.
/// </devdoc>
/// <summary>
/// Specifies the name of the application that is to be started, as well as a set
/// of command line arguments to pass to the application.
/// </summary>

public ProcessStartInfo(string fileName, IEnumerable<string> arguments)
{
_fileName = fileName;

foreach (string argument in arguments)
{
ArgumentList.Add(argument);
}
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding each argument individually, this could be:

Suggested change
foreach (string argument in arguments)
{
ArgumentList.Add(argument);
}
if (arguments is not null)
{
_argumentList = new Collection<string>(new List<string>(arguments));
}

and let List<T>'s ctor handle it however makes sense.

}

/// <devdoc>
/// Specifies the set of command line arguments to use when starting the application.
/// </devdoc>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1257,15 +1257,37 @@ public void UninitializedArgumentList()
}

[Fact]
public void InitializeWithArgumentList()
public void InitializeWithArgumentList_Add()
{
ProcessStartInfo psi = new ProcessStartInfo("filename");
psi.ArgumentList.Add("arg1");
psi.ArgumentList.Add("arg2");
psi.ArgumentList.Add(" arg3");
psi.ArgumentList.Add("arg4 ");
psi.ArgumentList.Add("arg 5");
psi.ArgumentList.Add($"arg{Environment.NewLine}6");

Assert.Equal(2, psi.ArgumentList.Count);
Assert.Equal(6, psi.ArgumentList.Count);
Assert.Equal("arg1", psi.ArgumentList[0]);
Assert.Equal("arg2", psi.ArgumentList[1]);
Assert.Equal(" arg3", psi.ArgumentList[2]);
Assert.Equal("arg4 ", psi.ArgumentList[3]);
Assert.Equal("arg 5", psi.ArgumentList[4]);
Assert.Equal($"arg{Environment.NewLine}6", psi.ArgumentList[5]);
}

[Fact]
public void InitializeWithArgumentList_Enumerable()
{
ProcessStartInfo psi = new ProcessStartInfo("filename", new[] { "arg1", "arg2", " arg3", "arg4 ", "arg 5", $"arg{Environment.NewLine}6" });

Assert.Equal(6, psi.ArgumentList.Count);
Assert.Equal("arg1", psi.ArgumentList[0]);
Assert.Equal("arg2", psi.ArgumentList[1]);
jbhensley marked this conversation as resolved.
Show resolved Hide resolved
Assert.Equal(" arg3", psi.ArgumentList[2]);
Assert.Equal("arg4 ", psi.ArgumentList[3]);
Assert.Equal("arg 5", psi.ArgumentList[4]);
Assert.Equal($"arg{Environment.NewLine}6", psi.ArgumentList[5]);
Copy link
Member

Choose a reason for hiding this comment

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

nit: it could be simplified

Suggested change
ProcessStartInfo psi = new ProcessStartInfo("filename", new[] { "arg1", "arg2", " arg3", "arg4 ", "arg 5", $"arg{Environment.NewLine}6" });
Assert.Equal(6, psi.ArgumentList.Count);
Assert.Equal("arg1", psi.ArgumentList[0]);
Assert.Equal("arg2", psi.ArgumentList[1]);
Assert.Equal(" arg3", psi.ArgumentList[2]);
Assert.Equal("arg4 ", psi.ArgumentList[3]);
Assert.Equal("arg 5", psi.ArgumentList[4]);
Assert.Equal($"arg{Environment.NewLine}6", psi.ArgumentList[5]);
string[] args = new[] { "arg1", "arg2", " arg3", "arg4 ", "arg 5", $"arg{Environment.NewLine}6" };
ProcessStartInfo psi = new ProcessStartInfo("filename", args);
Assert.Equal(args, psi.ArgumentList);

}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] // No Notepad on Nano
Expand Down