-
Notifications
You must be signed in to change notification settings - Fork 224
fix #2333 by adding a --framework switch #2363
Conversation
This has tooling impact /cc @BillHiebert @barrytang . How do we choose the .NET Framework from VS? |
auto frameworkName = GetOptionValue(argc, argv, _X("--framework")); | ||
if (frameworkName) | ||
{ | ||
SetEnvironmentVariable(_X("DNX_FRAMEWORK"), frameworkName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, default C++ formatting in VS is tabs. That explains why it looked fine in VS :P
Fixing
a163f1e
to
4d99131
Compare
@@ -176,7 +177,7 @@ void ExpandArgument(const dnx::char_t* value, std::vector<const dnx::char_t*>& e | |||
|
|||
bool ExpandCommandLineArguments(size_t nArgc, dnx::char_t** ppszArgv, size_t& nExpandedArgc, dnx::char_t**& ppszExpandedArgv) | |||
{ | |||
size_t pivot_parameter_idx = FindAppBaseOrNonHostOption(nArgc, ppszArgv); | |||
size_t pivot_parameter_idx = FindOption(nArgc, ppszArgv, _X("--appbase")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto? Looks like we want to use auto whenever possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to change this because it was already there but I had the same thought. I can just try it and see if it works :P
4d99131
to
34cb57a
Compare
34cb57a
to
e093baf
Compare
Yes we will need tooling support for this. Opened 1206194 What happens if the user specifies the core clr framework but is running on desktop (or vice versa)? I guess the tooling needs to restrict the framework choices to current DNX being run. |
The setting is only ever processed by Desktop CLR variants since they are the only DNXes that support multiple frameworks. I believe that specifying it to a CoreCLR variant is an error, but I will confirm. |
@@ -70,6 +70,9 @@ public static Task<int> ExecuteAsync(string[] args, FrameworkName targetFramewor | |||
CommandOptionType.MultipleValue); | |||
var optionDebug = app.Option("--debug", "Waits for the debugger to attach before beginning execution.", | |||
CommandOptionType.NoValue); | |||
#if DNX451 | |||
var optionFramework = app.Option("--framework <FRAMEWORK_ID>", "Set the framework version to use when running (i.e. dnx451, dnx452, dnx46, ...)", CommandOptionType.SingleValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should also not be displayed on Mono.
Fixes #2333
/cc @davidfowl @troydai @victorhurdugaci @BrennanConroy