Skip to content
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.

Sign up for GitHub

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

Format after building #1166

Closed
wants to merge 2 commits into from
Closed

Format after building #1166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cccece1
Format after building
TheAngryByrd Sep 17, 2023
1723ad8
Format on save
TheAngryByrd Sep 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .fantomasignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
test/
build/
benchmarks/
utils/
demo.fsx
Comment on lines +1 to +5
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replicated from Directory.Solution.targets. We should probably start considering formatting these at some point.

5 changes: 4 additions & 1 deletion .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,8 @@
"*.*proj": "msbuild",
"*.props": "msbuild",
"*.targets": "msbuild"
},
"[fsharp]": {
"editor.formatOnSave": true
Comment on lines +23 to +24
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not on save as well.

}
}
}
32 changes: 25 additions & 7 deletions Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
<Project>

<Target Name="SupportPrereleaseVersion"
AfterTargets="SetVersionFromChangelog">
<PropertyGroup Condition="'$(VersionSuffix)' != ''">
<Version>$(Version)-$(VersionSuffix)</Version>
<PackageVersion>$(Version)</PackageVersion>
</PropertyGroup>
</Target>
<PropertyGroup>
<_BuildProjBaseIntermediateOutputPath>$(MSBuildThisFileDirectory)build/obj/</_BuildProjBaseIntermediateOutputPath>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need some Chetfu here to make this not hardcoded to build's immediate output path but i'm not sure exactly how to ask for a specific project's value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is gonna be gross. I think you'll have to have a 'GetIntermediateOutputPathtarget in here that would return the value of theBaseIntermediateOutputPath` property for the project it was part of. Then you'd call that target on the build project during execution.

This dependency is why you often see features like this come in two targets:

  • one to set up any data dependencies and generate the properties/paths for any inputs/outputs to the real target
  • and the the real target the does the thing and uses the properties/items set up in the first target

this split is so that you always can set up your Inputs/Outputs and not run into issues with incrementality.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rainersigwald - we in the SDK get requests somewhat often for something like this: 'restore tools as part of my build'. Does anything here jump out as especially bad? Could we potentially extract some kind of recommendation in lieu of first class SDK level support?

<_DotnetToolManifestFile>$(MSBuildThisFileDirectory).config/dotnet-tools.json</_DotnetToolManifestFile>
<_DotnetToolRestoreOutputFile>$(_BuildProjBaseIntermediateOutputPath)/dotnet-tool-restore-$(NETCoreSdkVersion)</_DotnetToolRestoreOutputFile>
<_DotnetFantomasOutputFile>$(BaseIntermediateOutputPath)dotnet-fantomas-msbuild</_DotnetFantomasOutputFile>
</PropertyGroup>

<Target Name="SupportPrereleaseVersion"
AfterTargets="SetVersionFromChangelog">
<PropertyGroup Condition="'$(VersionSuffix)' != ''">
<Version>$(Version)-$(VersionSuffix)</Version>
<PackageVersion>$(Version)</PackageVersion>
</PropertyGroup>
</Target>

<!-- Make sure that dotnet tools (including paket) are restored before restoring any project -->
<Target Name="ToolRestore" BeforeTargets="Restore;CollectPackageReferences;PaketRestore" Inputs="$(_DotnetToolManifestFile)" Outputs="$(_DotnetToolRestoreOutputFile)">
<Exec Command="dotnet tool restore" WorkingDirectory="$(MSBuildThisFileDirectory)" StandardOutputImportance="High" StandardErrorImportance="High" />
<MakeDir Directories="$(_BuildProjBaseIntermediateOutputPath)"/>
<Touch Files="$(_DotnetToolRestoreOutputFile)" AlwaysCreate="True" ForceTouch="True" />
</Target>
Comment on lines +19 to +23
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inspired by FSharp.SystemTextJson but I worked out some more issues in MiniScaffold


<!-- Make sure that files are formatted before building -->
<Target Name="Format" BeforeTargets="BeforeBuild" Inputs="@(Compile)" Outputs="$(_DotnetFantomasOutputFile)" >
<Exec Command="dotnet fantomas $(MSBuildProjectDirectory)" StandardOutputImportance="High" StandardErrorImportance="High" WorkingDirectory="$(MSBuildThisFileDirectory)" />
<Touch Files="$(_DotnetFantomasOutputFile)" AlwaysCreate="True" ForceTouch="True" />
</Target>
Comment on lines +26 to +29
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does mean it runs per project, but it's incremental per project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the sane way to do it, since everything in MSBuild is project-based. Note that we would lose the ability to format any non-project-included files (but that's probably ok).

</Project>
8 changes: 3 additions & 5 deletions Directory.Solution.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
<Project>
<ItemGroup>
<FormatInputs Include="src/**/*.fs;src/**/*.fsi" Exclude="src/**/obj/**/*.fs" />
</ItemGroup>
Comment on lines -2 to -4
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using .fantomasignore



<Target Name="Format">
<Exec Command="dotnet fantomas @(FormatInputs, ' ') " />
<Exec Command="dotnet fantomas $(MSBuildThisFileDirectory)" />
</Target>

<Target Name="CheckFormat">
<Exec Command="dotnet fantomas --check @(FormatInputs, ' ') " />
<Exec Command="dotnet fantomas --check $(MSBuildThisFileDirectory) " />
</Target>
</Project>