-
Notifications
You must be signed in to change notification settings - Fork 790
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
Generate source for .resx files on build. #3607
Conversation
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.
LGTM (as a non-F# expert).
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.
Looks great.
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.
Requesting a few changes per comments
match item.GetMetadata(metadataName) with | ||
| value when String.IsNullOrWhiteSpace(value) -> defaultValue | ||
| value -> String.Compare(value, "true", StringComparison.OrdinalIgnoreCase) = 0 | ||
let generatedFiles, generatedResult = |
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.
Please check the latest code in https://github.com/Microsoft/visualfsharp/pull/3614/files#diff-f583ab0cc0cd86307b6b05f78682f65a for a cleaner way to write this. A general rule is not to use fold
unless you have a truly generative process where the results from one step depend on the results from the previous step
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.
Fixed. Extra commits coming soon.
try | ||
let printMessage = printfn "FSharpEmbedResXSource: %s" | ||
let justFileName = Path.GetFileNameWithoutExtension(resx) | ||
let namespaceName, moduleName = |
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.
Please check https://github.com/Microsoft/visualfsharp/pull/3614/files#diff-f583ab0cc0cd86307b6b05f78682f65a for the hack I put in to make sure this doesn't repeatedly re-generate content, causing unexpected rebuilds on a simple repeated build like build net40
. If there's a better, more declarative MSBuild way to do it without the file check comparison in the code then please let's do that.
Please also manually check that repeated build net40
doesn't rebuild.
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.
Fixed. Extra commits coming soon.
The error output should be the exception message, not the exception type.
|> FSharpSR.GetStringWithCR | ||
| Above -> FSharpSR.FileCannotBePlacedBodyAbove() | ||
| Below -> FSharpSR.FileCannotBePlacedBodyBelow() | ||
|> (fun s -> s.Replace("\n", Environment.NewLine)) |
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.
This is incorrect - this should be @"\n"
.
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.
Good catch.
null, | ||
OLEMSGICON.OLEMSGICON_INFO, OLEMSGBUTTON.OLEMSGBUTTON_OK, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST | ||
) |> ignore | ||
Marshal.ThrowExceptionForHR(VSConstants.OLE_E_PROMPTSAVECANCELLED) | ||
let result = | ||
VsShellUtilities.ShowMessageBox(node.Site, FSharpSR.GetStringWithCR(FSharpSR.NeedReloadToChangeTargetFx), | ||
VsShellUtilities.ShowMessageBox(node.Site, FSharpSR.NeedReloadToChangeTargetFx().Replace("\n", Environment.NewLine), |
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.
This is incorrect - this should be @"\n"
.
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 don't know how many times I have made that same mistake over the years. Good eyes.
|
||
namespace Microsoft.VisualStudio.FSharp.Interactive | ||
|
||
type DisplayNameAttribute(resName) = |
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.
Can we give these a different name? It's confusing looking at fsiLanguageService.fs
that they don't point to the System.ComponentModel attributes.
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'm in favor of renaming them, but to keep the scope of this PR small (e.g., code generation for resources) I'd like to leave the names as they stand.
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.
@brett: perhaps ResourceCaregoryAttribute, ResourceDescriptionAttribute and ResourceDisplayNameAttribute.
|> Seq.fold (fun (sb:StringBuilder) (node:XElement) -> | ||
let name = | ||
match node.Attribute(xname "name") with | ||
| null -> failwith "Missing resource name" |
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.
Can we provide a more helpful message here?
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.
Fixed. Updated commits coming soon.
let getBooleanMetadata (metadataName:string) (defaultValue:bool) (item:ITaskItem) = | ||
match item.GetMetadata(metadataName) with | ||
| value when String.IsNullOrWhiteSpace(value) -> defaultValue | ||
| value -> String.Compare(value, "true", StringComparison.OrdinalIgnoreCase) = 0 |
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.
Throw if the result isn't false? e.g. someone could write "tru e"
and it would be interpreted as false.
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.
Good idea. Updated commits coming soon.
let accessorBody = | ||
match (generateLegacy, generateLiteral) with | ||
| (true, true) -> sprintf " [<Literal>]\n let %s = \"%s\"" identifier name | ||
| (true, false) -> sprintf " let %s = \"%s\"" identifier name // the [<Literal>] attribute can't be used for FSharp.Core |
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.
Can we not automatically detect if we're generating for FSharp.Core?
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.
Not directly. This generator only has access to whatever is in the project file, and while we could detect FSharp.Core.fsproj
by either $(ProjectName)
or the existence of the FSHARP_CORE
constant, these are merely conventions that could change and it seems wrong to assume every project named FSharp.Core
or every project with a defined constant FSHARP_CORE
is our FSharp.Core.
| null -> true | ||
| _ -> false | ||
match (isStringResource, generateGetObject) with | ||
| (true, _) -> sprintf " let %s() = GetString(\"%s\")" identifier name |
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.
Just a thought, but could we replace @"\n"
with "\n"
at this point? Also it would be really neat if we could detect {0},{1}
etc. in the string and then make a function take them as parameters. Such as:
let MyString (parameter1 : string) = String.Format(GetString("MyString"), parameter1)
It means string formatting identifiers in the resx will fail the build if they aren't expanded.
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.
Regarding the @"\n" -> "\n"
replacement, I'd rather not do it here because I would like this to be as close to the C#/VB implementation as possible and that only outputs ResourceManager.GetString()
. Also, it seems wrong to do any resource manipulation without the call site explicitly knowing about it, especially since each fetch of a string resource now contains an extra function call and potentially more string allocations.
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.
Some of the SR modules that you removed did this already - although am I right in saying that this code is for all FSharp projects, not just VFT projects?
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.
As for detecting {0},{1}
, that's a surprisingly hard problem. In the past a Roslyn team member tried to write a C#/VB analyzer to detect mismatches between the value to String.Format()
and the replacement values. That ended up being abandoned because to make it correct the entire string parsing had to be re-implemented and it gets very nasty very fast.
module internal {1} = | ||
type private C (_dummy:System.Object) = class end | ||
let mutable Culture = System.Globalization.CultureInfo.CurrentUICulture | ||
let ResourceManager = new System.Resources.ResourceManager(""{2}"", C(null).GetType().GetTypeInfo().Assembly) |
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.
Can't this be replaced with Assembly.GetCallingAssembly
?
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's the current executing assembly that is interesting not the calling assembly. This irritating code is the magic formulation because GetExecutingAssembly() didn\t make it into coreclr until V2.0, and the buildtask is compiled for both the desktop and coreclr.
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.
In which case can we make the C
class simpler:
type private Dummy = interface end
...
let ResourceManager = new System.Resources.ResourceManager(""{2}"", typeof<Dummy>.Assembly)
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.
@saul sure.
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.
We can't use typeof<>
because this generator is used by FSharp.Core
and at the point that we need these strings is before the typeof<>
operator can be defined (i.e., in prim-types.fs/fsi
). I could do a custom switch on FSharp.Core
, but that seemed like a nasty hack given that the hack already there will always work.
Edit: and the .GetType().GetTypeInfo()
method works in both desktop and CoreCLR scenarios and I'd rather have one method that always works than switching on platform for something that doesn't matter to the end user.
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.
@brettfo thanks for clearing that up, I couldn't for the life of me remember why we did it this way.
if parts.Length = 1 then ("global", parts.[0]) | ||
else (String.Join(".", parts, 0, parts.Length - 1), parts.[parts.Length - 1]) | ||
let generateGetObject = | ||
match _targetFramework with |
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.
netcoreapp1.* as well
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.
Thanks, updated commits coming.
…reapp1.*` platforms
@dotnet-bot test this please (ci_part2 timed out) |
I've addressed all feedback received so far, ping for further review. |
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.
Please consider renaming those attributes.
|
||
namespace Microsoft.VisualStudio.FSharp.Interactive | ||
|
||
type DisplayNameAttribute(resName) = |
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.
@brett: perhaps ResourceCaregoryAttribute, ResourceDescriptionAttribute and ResourceDisplayNameAttribute.
@brettfo, @KevinRansom or @dsyme, is it possible that this merge has caused #3638? It seems related, but I am surprised that the build server or CI process didn't notice this, so perhaps it is caused by something more local, though I have tried a rebuild on a clean master which resulted in the same errors. |
* Generate source for .resx files on build. (#3607) * add build task to generate *.fs from *.resx files * generate source for embedded resources in tests * generate source for embedded resources in FSharp.Editor * generate source for embedded resources in FSharp.LanguageService * generate source for embedded resources in FSharp.ProjectSystem.FSharp * generate source for embedded resources in FSharp.VS.FSI * don't generate non-string resources when <=netstandard1.6 * update baseline error message for tests The error output should be the exception message, not the exception type. * perform up-to-date check before generating *.fs from *.resx * remove non-idiomatic fold for an array comprehension * correct newline replacement * output more friendly error message * throw if boolean value isn't explicitly `true` or `false` * only generate object resource code on non `netstandard1.*` and `netcoreapp1.*` platforms * ensure FSharp.Core specifies a target framework for resource generaton * rename attributes to be non-ambiguous and properly include them * fix order of file items in FSharp.Core * Fix build.cmd for certain always-shown errors (like "unable to find registry key"), improve finding of VS2017 tools, fix DEVGUIDE.md (#3635) * Fix build.com displaying the following error on each run "ERROR: The system was unable to find the specified registry key or value." * Remove warning about reg.exe errors introduced in #3614 (in commit b548bd7, but unrelated to that fix), it is no longer necessary. * Fix #3638, VS2017 Preview installation was not found when VS2017RTM is not installed. Remove comment introduced in #3614 (through commit 966bd7f) * Fixing JaroWinkler tests with InvariantCulture and fixing async tests by removing Debugger.Break() (#3627) * Fixing JaroWinkler tests to use InvariantCulture for number-to-string * Fixing the crashing of test runners because of a Debugger.Break() in a test * update to System.Collections.Immutable 1.3.1 (#3641) * update to System.Collections.Immutable 1.3.1 * fixes * fix assembly reference * [WIP] Adds optimized emit for int64 constants (#3620) * Adds optimized emit for int64 constants. * Adds comment linking to the changing PR. Thanks for this PR. Kevin * fix assembly reference (#3646) * Remove a few more build warnings (#3647) * fix assembly reference * remove more build warnings * fix build * move BuildFromSource projects to their own directory * Adds tests for emitted IL for new Int64 constants. (#3650) * Enable FS as prefix and ignore invalid values for warnings (#3631) * enable fs as prefix and ignore invalid values for warnings + tests * Allow #pragma to validate warnings * do it right * use ordinal compare * In both places * Add fs prefix to warnaserror * Fixup tests * Fix stack overflow on assembly resolution (#3658) * Fix stack overflow on tp assembly resolution * Feedback * Add impl files to file check results (#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (#3657) * bump FCS version (#3676) * bump version * Update RELEASE_NOTES.md * Parsing improvements: no reactor, add parsing options, error severity options (#3601) * Parse without reactor, add parsing options, error severity options * Revert parsing APIs (fallback to the new ones), fix VFT projects * Cache parse results after type check * Add impl files to file check results (#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (#3657) * bump FCS version (#3676) * bump version * Update RELEASE_NOTES.md * updates to make tests pass * restore old behaviour of CheckFileInProjectAllowingStaleCachedResults (builder had been created by ParseFileInProject) * restore use of CheckFileInProjectAllowingStaleCachedResults * deprecate test relying on whacky behaviour of deprecated GetCheckResultsBeforeFileInProjectEvenIfStale * Use ParseFile and FSharpParsingOptions instead of ParseFileInProject * prepare FCS release with this feature * whitespace cleanup (#3682) * whitespace and docs (#3684) * Preserve XML docs for in-memory project references (#3683) * fix xmldocs for in-memory project references * add test * fix tests * whitespace and comments (#3686) * fix assembly reference * whitespace and comments * whitespace and comments * whitespace and comments * cherry pick two PRs from FCS (#3687) * fix assembly reference * remove line endings from all *.nuspec files * ProjectCracker returns *.fsi files in FSharpProjectOptions.SourceFiles array (in addition to *.fs files, in right order) * ProjectCracker raises exception if ProjectCrackerTool returns non null ProjectCrackerOptions.Error (new field) * fix build on linux * fix a test * slashes * revert slashes * Update FSharp.Compiler.Service.ProjectCracker.nuspec * try to fix travis * try to fix travis * list dependencies * no obsolete pdb in nuget * list dependencies * cherry pick of fsharp/fsharp change * bump FCS version number (#3688) * Update FSharp.Compiler.Service.MSBuild.v12.nuspec * fix FCS nuget on windows * fix-resource (#3690) * Bump FSharp.Compiler.Tools to 4.1.27 and align mono build files (#3693) * ri change from fsharp * fix test * bump FSC tools to 4.1.27 * remove fsharp.core from Mono GAC * align mono directory * fix typo * install back versions with Mono * fix typo * update FCS doc generation (#3694) * update DEVGUIDE to add addiitional steps before running build (#3725) * Split templates out into a seperate vsix (#3720) * merge error * Merge issues
* add build task to generate *.fs from *.resx files * generate source for embedded resources in tests * generate source for embedded resources in FSharp.Editor * generate source for embedded resources in FSharp.LanguageService * generate source for embedded resources in FSharp.ProjectSystem.FSharp * generate source for embedded resources in FSharp.VS.FSI * don't generate non-string resources when <=netstandard1.6 * update baseline error message for tests The error output should be the exception message, not the exception type. * perform up-to-date check before generating *.fs from *.resx * remove non-idiomatic fold for an array comprehension * correct newline replacement * output more friendly error message * throw if boolean value isn't explicitly `true` or `false` * only generate object resource code on non `netstandard1.*` and `netcoreapp1.*` platforms * ensure FSharp.Core specifies a target framework for resource generaton * rename attributes to be non-ambiguous and properly include them
* Generate source for .resx files on build. (dotnet#3607) * add build task to generate *.fs from *.resx files * generate source for embedded resources in tests * generate source for embedded resources in FSharp.Editor * generate source for embedded resources in FSharp.LanguageService * generate source for embedded resources in FSharp.ProjectSystem.FSharp * generate source for embedded resources in FSharp.VS.FSI * don't generate non-string resources when <=netstandard1.6 * update baseline error message for tests The error output should be the exception message, not the exception type. * perform up-to-date check before generating *.fs from *.resx * remove non-idiomatic fold for an array comprehension * correct newline replacement * output more friendly error message * throw if boolean value isn't explicitly `true` or `false` * only generate object resource code on non `netstandard1.*` and `netcoreapp1.*` platforms * ensure FSharp.Core specifies a target framework for resource generaton * rename attributes to be non-ambiguous and properly include them * fix order of file items in FSharp.Core * Fix build.cmd for certain always-shown errors (like "unable to find registry key"), improve finding of VS2017 tools, fix DEVGUIDE.md (dotnet#3635) * Fix build.com displaying the following error on each run "ERROR: The system was unable to find the specified registry key or value." * Remove warning about reg.exe errors introduced in dotnet#3614 (in commit b548bd7, but unrelated to that fix), it is no longer necessary. * Fix dotnet#3638, VS2017 Preview installation was not found when VS2017RTM is not installed. Remove comment introduced in dotnet#3614 (through commit 966bd7f) * Fixing JaroWinkler tests with InvariantCulture and fixing async tests by removing Debugger.Break() (dotnet#3627) * Fixing JaroWinkler tests to use InvariantCulture for number-to-string * Fixing the crashing of test runners because of a Debugger.Break() in a test * update to System.Collections.Immutable 1.3.1 (dotnet#3641) * update to System.Collections.Immutable 1.3.1 * fixes * fix assembly reference * [WIP] Adds optimized emit for int64 constants (dotnet#3620) * Adds optimized emit for int64 constants. * Adds comment linking to the changing PR. Thanks for this PR. Kevin * fix assembly reference (dotnet#3646) * Remove a few more build warnings (dotnet#3647) * fix assembly reference * remove more build warnings * fix build * move BuildFromSource projects to their own directory * Adds tests for emitted IL for new Int64 constants. (dotnet#3650) * Enable FS as prefix and ignore invalid values for warnings (dotnet#3631) * enable fs as prefix and ignore invalid values for warnings + tests * Allow #pragma to validate warnings * do it right * use ordinal compare * In both places * Add fs prefix to warnaserror * Fixup tests * Fix stack overflow on assembly resolution (dotnet#3658) * Fix stack overflow on tp assembly resolution * Feedback * Add impl files to file check results (dotnet#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (dotnet#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (dotnet#3657) * bump FCS version (dotnet#3676) * bump version * Update RELEASE_NOTES.md * Parsing improvements: no reactor, add parsing options, error severity options (dotnet#3601) * Parse without reactor, add parsing options, error severity options * Revert parsing APIs (fallback to the new ones), fix VFT projects * Cache parse results after type check * Add impl files to file check results (dotnet#3659) * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project * add ImplementationFiles to FSharpCheckFileResults * make FSharpImplementationFileContents ctor internal * throw if ImplementationFiles is called having keepAssemblyContents flag set to false * add a test * spelling and cosmetics * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd (dotnet#3672) * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * This adds backup, restore, coloration and many more checks to the update-vsintegration.cmd * Remove ambiguous an irrelevant instruction, improved help and instructions * Fix a scenario where the return code wasn't nonzero for error conditions, fixes not creating backup dir when not backing up * add LanguageServiceProfiling project to internals visible to list of FSharp.Compiler.Private project (dotnet#3657) * bump FCS version (dotnet#3676) * bump version * Update RELEASE_NOTES.md * updates to make tests pass * restore old behaviour of CheckFileInProjectAllowingStaleCachedResults (builder had been created by ParseFileInProject) * restore use of CheckFileInProjectAllowingStaleCachedResults * deprecate test relying on whacky behaviour of deprecated GetCheckResultsBeforeFileInProjectEvenIfStale * Use ParseFile and FSharpParsingOptions instead of ParseFileInProject * prepare FCS release with this feature * whitespace cleanup (dotnet#3682) * whitespace and docs (dotnet#3684) * Preserve XML docs for in-memory project references (dotnet#3683) * fix xmldocs for in-memory project references * add test * fix tests * whitespace and comments (dotnet#3686) * fix assembly reference * whitespace and comments * whitespace and comments * whitespace and comments * cherry pick two PRs from FCS (dotnet#3687) * fix assembly reference * remove line endings from all *.nuspec files * ProjectCracker returns *.fsi files in FSharpProjectOptions.SourceFiles array (in addition to *.fs files, in right order) * ProjectCracker raises exception if ProjectCrackerTool returns non null ProjectCrackerOptions.Error (new field) * fix build on linux * fix a test * slashes * revert slashes * Update FSharp.Compiler.Service.ProjectCracker.nuspec * try to fix travis * try to fix travis * list dependencies * no obsolete pdb in nuget * list dependencies * cherry pick of fsharp/fsharp change * bump FCS version number (dotnet#3688) * Update FSharp.Compiler.Service.MSBuild.v12.nuspec * fix FCS nuget on windows * fix-resource (dotnet#3690) * Bump FSharp.Compiler.Tools to 4.1.27 and align mono build files (dotnet#3693) * ri change from fsharp * fix test * bump FSC tools to 4.1.27 * remove fsharp.core from Mono GAC * align mono directory * fix typo * install back versions with Mono * fix typo * update FCS doc generation (dotnet#3694) * update DEVGUIDE to add addiitional steps before running build (dotnet#3725) * Split templates out into a seperate vsix (dotnet#3720) * merge error * Merge issues
This PR auto-generates the code necessary to access resources from a
.resx
file automatically during build. Included in this is the deletion of a lot of manually maintained code.This is added as build targets/tasks in
FSharp.Build
for two reasons:resgen.exe
would take forever for it to find its way to F# builds.dotnet build
.There are a few items to point out:
GenerateSource
metadata which can be specified in one of two ways: inline<EmbeddedResource Include="file.resx" GenerateSource="true" />
, or as a child element:<EmbeddedResource Include="file.resx"><GenerateSource>true</GenerateSource></EmbeddedResource>
. Where possible I've used the first attribute-based method, but some projects built with xbuild on Linux don't allow extra attributes, so for those projects I've resorted to the element approach.GeneratedModuleName
metadata when supplied, otherwise I do what resgen does for C#/VB and use the file name as the base. This is particularly useful inFSharp.Core
where I want the generated code to continue to be namedMicrosoft.FSharp.Core.SR
but can't rename the resource file toMicrosoft.FSharp.Core.SR.resx
because that's incorrectly determined to be Serbian resources ("sr"
).unit -> string
, but this doesn't work forFSharp.Core
because the typeunit
hasn't been defined by the time some of the strings are needed. Attempting to move the definition ofUnit/unit
toprim-types-prelude.fs/fsi
was unsuccessful due to type matching (:?
operator) not being defined yet which is needed in the.Equals()
overload. The solution to this was to allow generation of code nearly identical to the deletedSR.fs
by defining string constants instead of functions and manually passing those through.GetString()
. This is handled by theGenerateLegacyCode
metadata which defaults tofalse
.[<Literal>]
attribute because that's needed in some of the VS-specific resources, but again this can't be done inFSharp.Core
because that attribute doesn't yet exist, so this is controlled by theGenerateLiterals
metadata which defaults totrue
.Not implemented in this PR:
unit -> obj
, but eventually it would be nice to parse thetype
attribute and the content of the<value>
element to properly generate a function with the signatureunit -> System.Drawing.Bitmap
.