-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix dotnet run double dash passing arguments #6313
Conversation
@dotnet/dotnet-cli @jonsequitur @mikeharder Fixes dotnet/sdk#1118 |
@@ -0,0 +1,16 @@ | |||
<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
e42700d
to
d6c1609
Compare
Args = args; | ||
} | ||
|
||
public RunCommand MakeNewWithReplaced(string configuration = null, |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
d6c1609
to
720de6c
Compare
src/dotnet/ParserExtensions.cs
Outdated
@@ -9,6 +9,8 @@ public static class ParserExtensions | |||
this CommandLine.Parser parser, | |||
string context, | |||
string[] args) => | |||
parser.Parse(context.Split(' ').Concat(args).ToArray()); | |||
parser.Parse(context.Split(' ') | |||
.Concat(args.SelectMany(a => a.Tokenize())) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
dcd5f4d
to
9ae6d79
Compare
When run “dotnet run -- foo”, foo should be the argument passed to the subject app. After replacing the original parser, dotnet-run did not utilize the “unparsedtoken” of the parsed result. To append unparsedtoken to RunCommand’s argument is not straight forward. RunCommand has an “immutable constructor”, which is a good thing, so I made update RunCommand’s argument following the immutable pattern -- create a new object with the original field but only change the arguments. I also made these filed private set.
9ae6d79
to
83f3a3e
Compare
@wli3 @livarcocc please add ask mode template and then tag me for approval. |
Customer scenario In dotnet run command
However, there is a regression for this feature. The arguments cannot be passed to the subject application. Bugs this fixes Workarounds, if any no workaround for this issue. Risk As the issue stated, this bug is blocking partner. It could also cause customer CI scripts disfunction. Performance impact Low - only few in-memory object manipulation added. Root cause analysis CLI start adapting new command line parser in March, 2017, the conversion for the run command failed to utilize the parse result. No test coverage for dotnet-run double dash scenario. How was the bug found? manual testing |
@@ -9,6 +9,10 @@ public class Program | |||
{ | |||
public static void Main(string[] args) | |||
{ | |||
if (args.Length > 0) | |||
{ |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -24,6 +24,11 @@ public static int Run(string[] args) | |||
|
|||
Console.WriteLine(result.Diagram()); | |||
|
|||
if (result.UnparsedTokens.Any()) { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
90de12e
to
def4322
Compare
@dotnet-bot Test Windows_NT x86 Debug Build |
When run “dotnet run -- foo”, foo should be the argument passed to the
subject app. After replacing the original parser, dotnet-run did not
utilize the “unparsedtoken” of the parsed result.
There are several more issues
Due to dotnet top command are not using the new parser, the actual token
passed to the new parser at dotnet-run level are “dotnet” “run” “ --
foo” which will not give the correct result. The tokens need to be
further tokenized to be “dotnet” “run” “--” “foo”.
To append unparsedtoken to RunCommand’s argument is not straight
forward. RunCommand has an “immutable constructor”, which is a good
thing, so I made update RunCommand’s argument following the immutable
pattern -- create a new object with the original field but only change
the arguments.