make command line struct's .ctors public #18258
Conversation
|
CC @dotnet/roslyn-compiler I know we need to unblock F# here but want to get a quick read from @gafter about making these part of the public API surface. They look fairly innocuous but guessing there is a reason they were initially made non-public. If there is some content here I'd be happy with letting this go through + file an issue no find the appropriate API here before 15.3 shipped. |
| @@ -21,12 +21,12 @@ namespace Microsoft.CodeAnalysis | |||
| /// </summary> | |||
| public abstract class CommandLineArguments | |||
| { | |||
| internal bool IsScriptRunner { get; set; } | |||
| public bool IsScriptRunner { get; protected internal set; } | |||
There was a problem hiding this comment.
💡 This property should be documented since it's being moved to the public API surface for the first time.
brettfo
force-pushed
the
expose-command-line-parser
branch
from
e1f8995
to
a57ec5a
Compare
|
Looking further into this, I can rework and simplify some stuff in the project system and that would only require making two constructors public. I think this is the better and correct approach because similar |
brettfo
changed the title
|
// Debug vs integration tests failed, log is here. @dotnet-bot test this please |
|
Ping @jaredpar on opening these two struct .ctor()s. |
|
CC @AnthonyDGreen in case he wants to push back but this seems fine to me. |
|
The proposed solution seems fine on its face. I would prefer it if VB, C#, and F# could all use the same methods of extensibility. If we're going to make them protected internal why not just make them protected? Do VB and C# need greater access than F# for some reason? |
|
tl;dr - I spoke with @AnthonyDGreen and he's OK with this. |
|
@brettfo With the latest design for hooking F# up, do we still need this? |
|
Yes, these constructors are used here and again on line 47. |
EDIT:
I've re-thought this and I've come up with a simpler solution. See the explanation in my comment here.
(Original text)
To integrate F# with the project system, F# needs to provide an implementation of
CommandLineArguments, but that has some internal-only setters. This PR simply makes thoseprotected internal, instead so they're consumable without adding IVTs.@dotnet/roslyn-compiler and specifically, @jaredpar and @jasonmalinowski.