safesparrow / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
1 stars 0 forks source link

Refactorings #37

Closed safesparrow closed 1 year ago

safesparrow commented 1 year ago

I didn't use Fantomas, because when I ran it, I got changes in 25 files and also in lines I didn't modify, so I wasn't sure which of those I was supposed to commit.

nojaf commented 1 year ago

I can't seem to build the project when checking out this branch: image

Did you push everything?

nojaf commented 1 year ago

I didn't use Fantomas, because when I ran it, I got changes in 25 files and also in lines I didn't modify, so I wasn't sure which of those I was supposed to commit.

If unrelated files changed, I might mean that you were invoking Fantomas from another location than the repository root. TL;DR there are known issues with the .fantomasignore file, it only works properly if you run dotnet fantomas src -r from the repository root.

nojaf commented 1 year ago

scenario "A open statement that leads nowhere should link to every file that came above it." failed locally for me. This test covers ghost dependencies specifically.

safesparrow commented 1 year ago

But what really disturbed me was that none of my local projects were running in graph mode. Because --deterministic+ is enabled by default and you need to explicitly turn that off before graph can kick in. Only when setting --deterministic- will --test:GraphBasedChecking do anything. This should be added for the methodOptions in CompliationTests.fs, but maybe we also want to print a warning when --deterministic+ is combined with --test:GraphBasedChecking.

This is something I should fix in the main branch (or here) - perhaps when the feature is enabled via the dedicated flag, it should stay enabled even in deterministic builds, and only the experimental flag should have the implicit disable possible.

(Or if we manage to solve determinism we can revert this altogether)

safesparrow commented 1 year ago

I reverted controversial changes and fixed the build.

If unrelated files changed, I might mean that you were invoking Fantomas from another location than the repository root. TL;DR there are known issues with the .fantomasignore file, it only works properly if you run dotnet fantomas src -r from the repository root.

I ran the following:

PS C:\projekty\fsharp\nojaf> dotnet tool run fantomas --version
Fantomas v5.0.3+f1ec0088672e4b075aab51d5b66a99c7ae538260

PS C:\projekty\fsharp\nojaf> git reset --hard
PS C:\projekty\fsharp\nojaf> dotnet tool run fantomas -r .

and this gives the following result:

PS C:\projekty\fsharp\nojaf> git status
On branch graph-tc-new-tweaks
Your branch is up to date with 'safesparrow/graph-tc-new-tweaks'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   vsintegration/tests/FSharp.Editor.Tests/BraceMatchingServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/BreakpointResolutionServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/DocumentDiagnosticAnalyzerTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/DocumentHighlightsServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/EditorFormattingServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/FsxCompletionProviderTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/HelpContextServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/Hints/HintTestFramework.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/Hints/InlineTypeHintTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/IndentationServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/LanguageDebugInfoServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/QuickInfoProviderTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/QuickInfoTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/SemanticClassificationServiceTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/SignatureHelpProviderTests.fs
        modified:   vsintegration/tests/FSharp.Editor.Tests/SyntacticColorizationServiceTests.fs

I'll keep reverting those changes, but I can't see what I'm doing wrong.

nojaf commented 1 year ago

Oh, that is interesting that. I always run Fantomas on the src folder. vsintegration is listed in the .fantomasignore but I'm not picked up. I'm not sure if that is to be expected or not. Maybe they shuffled stuff around. I'll look into this.