[WIP] add F# WriteCodeFragment #2132
Conversation
|
Thanks for opening a PR. Unfortunately we've already closed a similar PR (#2058) after expressing concerns that we would not like to have this logic in MSBuild. If you think this is not correct please open an issue to discuss. |
|
@AndyGerlicher while long term this is not the best place, atm the So maybe is possibile to make a special case for F# in @AndyGerlicher i need to open an issue to discuss, or can be discussed there? /cc @KevinRansom @cartermp |
|
Adding @dsplaisted @eerhardt for an hint about this, because F# now is not supported in Dunno if there is a better place to discuss, but pratically all the code needed is already there (i just need to add more unit tests, atm is just one), so maybe easier to see on what adding this mean for msbuild @AndyGerlicher |
|
in our targets file and sdk apps will work fine. But we should also send a PR to the dotnet sdk refactoring the current implementation a bit. The bit where the attributes are generated should be moved to a new target: GetAssemblyAttributes or some such name. And then : CoreGenerateAssemblyInfo just handles the write. We can override CoreGenerateAssemblyInfo to write the attributes with a build task added to the Microsoft.FSharp.Build library ... or something. Does that make sense? Enrico, Kevin |
There's already dotnet/sdk#1007 which splits the target into |
|
Awesome @dasMulli , can you please add me and @KevinRansom and reference the f# issue dotnet/fsharp#3113 in that PR, so we'll update. |
| @@ -641,6 +641,32 @@ public void OneAttributePositionalAndNamedParamsVisualBasic() | |||
| File.Delete(task.OutputFile.ItemSpec); | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Test with the VB language | |||
| string content = File.ReadAllText(task.OutputFile.ItemSpec); | ||
| Console.WriteLine(content); | ||
|
|
||
| CheckContentFSharp(content, "[<assembly: System.AssemblyTrademarkAttribute()>]"); |
There was a problem hiding this comment.
Maybe test with multiple attributes and attributes with parameters?
| @@ -152,6 +152,12 @@ public override bool Execute() | |||
| [SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.IO.StringWriter.#ctor(System.Text.StringBuilder)", Justification = "Reads fine to me")] | |||
| private string GenerateCode(out string extension) | |||
| { | |||
| if (Language.ToLowerInvariant() == "f#") | |||
| { | |||
| //no codedoom for F#, fallback to coreclr version | |||
|
Thx for review @saul but this is going to Ben replaced with another, without writecodegragment |
ref dotnet/netcorecli-fsc#93
to generate the
assemblyinfo.fsfrom fsproj properties.@KevinRansom the plan is to do it like c# does for coreclr (where codedom doesnt exists anyway), like here in src/Tasks/WriteCodeFragment.cs#L294-L313
/cc @rainersigwald