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.

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

Honor .fantomasignore file when processing a folder #1834

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

kentcb
Copy link
Contributor

@kentcb kentcb commented Jul 19, 2021

When processing an entire folder with a command such as this:

fantomas --recurse .

the .fantomasignore file is effectively useless because the processSourceString function is generating a fixed file name of either tmp.fs or tmp.fsi. This file name is in turn used by a call to IgnoreFile.isIgnoredFile by formatContentInternalAsync, so a .fantomasignore file like this presents an issue:

*.fs

!/path/to/important.fs

Even when formatting important.fs, it will be "seen" as tmp.fs by formatContentInternalAsync and therefore ignored.

This PR is an intuitive attempt at fixing the issue with minimal change. That said, I do feel like the ignore check is possibly occurring in the wrong place and would be better hoisted to an outer layer of the system. In fact, Program.fs seemingly already does the appropriate ignore checks, so the secondary check within formatContentInternalAsync seems redundant in this context. However, I haven't enough experience with this system to know that one way or another without a lot of digging (open to feedback on that).

@nojaf
Copy link
Contributor

nojaf commented Jul 19, 2021

I haven't checked your PR yet, but claiming .fantomasignore is not respected when passing in a folder seems like a bold statement.
I would expect the flow to be:

| InputPath.Folder p1, OutputPath.Notknown -> processFolder p1 p1

let processFolder inputFolder outputFolder =

let rec allFiles isRec path =
let searchOption =
(if isRec then
SearchOption.AllDirectories
else
SearchOption.TopDirectoryOnly)
Directory.GetFiles(path, "*.*", searchOption)
|> Seq.filter
(fun f ->
isFSharpFile f
&& not (isInExcludedDir f)
&& not (IgnoreFile.isIgnoredFile f))

So, the check being IgnoreFile.isIgnoredFile f.

The problem you most likely have is probably related to markashleybell/MAB.DotIgnore#9 (comment).

@kentcb
Copy link
Contributor Author

kentcb commented Jul 19, 2021

Please see my PR, which includes tests to prove my bold statement :)

This is the (second) check that fails:

if IgnoreFile.isIgnoredFile file then

And it's using the file name determined by:

let fileName =
if isFsiFile then
"/tmp.fsi"
else
"/tmp.fsx"

Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

I stand corrected, if the file renaming happens in processSourceString, we cannot do the ignore check in formatContentInternalAsync with the renamed file.
Thank you for this contribution!

@nojaf nojaf merged commit 39decbe into fsprojects:master Jul 20, 2021
knocte added a commit to nblockchain/fantomless that referenced this pull request Sep 20, 2021
It is failing with:

Microsoft (R) Build Engine version 16.11.0+0538acc04 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

/Users/runner/.dotnet/sdk/5.0.400/MSBuild.dll -maxcpucount -property:Configuration=Release -property:PackageOutputPath=/Users/runner/work/fantomless/fantomless/bin /p:Title=Fantomas /p:PackageVersion=4.5.1 /p:Authors=Florian Verdonck Jindřich Ivánek /p:Owners=Anh-Dung Phan /p:PackageRequireLicenseAcceptance=false /p:Description=This library aims at formatting F# source files based on a given configuration.
Fantomas will ensure correct indentation and consistent spacing between elements in the source files.
Some common use cases include
(1) Reformatting a code base to conform a universal page width
(2) Converting legacy code from verbose syntax to light syntax
(3) Formatting auto-generated F# signatures. /p:Summary=Source code formatter for F# /p:PackageReleaseNotes=Fix StackOverflow exceptions when collecting ColMultilineItem list. [fsprojects#1839](fsprojects#1839)
Honor .fantomasignore file when processing a folder. [fsprojects#1834](fsprojects#1834)
Fix Overly indented members on a record type with accessibility modifier. [fsprojects#1824](fsprojects#1824)
Fix MultiLineLambdaClosingNewline not respected with function keyword. [fsprojects#1823](fsprojects#1823)
Fix Comment is lost at the end of a match. [fsprojects#1822](fsprojects#1822) /p:Copyright=Copyright © 2021 /p:PackageTags=F# fsharp formatting beautifier indentation indenter /p:PackageProjectUrl=https://github.com/fsprojects/fantomas -target:pack -verbosity:m /bl:/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/tmpF67x0C.tmp.binlog /clp:ForceConsoleColor src/Fantomas/Fantomas.fsproj
  Fantomas -> /Users/runner/work/fantomless/fantomless/src/Fantomas/bin/Release/netstandard2.0/Fantomas.dll
  Paket version 6.0.0-rc006
  Total time taken: 0 milliseconds
/Users/runner/work/fantomless/fantomless/.paket/Paket.Restore.targets(370,15): error MSB4030: " " is an invalid value for the "NoDefaultExcludes" parameter of the "PackTask" task. The "NoDefaultExcludes" parameter is of type "System.Boolean". [/Users/runner/work/fantomless/fantomless/src/Fantomas/Fantomas.fsproj]
: /Users/runner/work/fantomless/fantomless/.paket/Paket.Restore.targets(370,15): error MSB4030: " " is an invalid value for the "NoDefaultExcludes" parameter of the "PackTask" task. The "NoDefaultExcludes" parameter is of type "System.Boolean".
Finished (Failed) 'DotNet:pack' in 00:00:17.2484182
Finished (Failed) 'Pack' in 00:00:17.2537065

---------------------------------------------------------------------
Build Time Report
---------------------------------------------------------------------
Target           Duration
------           --------
Clean            00:00:00.0054029
ProjectVersion   00:00:00.0095845
CheckFormat      00:00:28.1530106
Build            00:00:50.1780681
UnitTests        00:00:52.8291326
Benchmark        00:01:18.1282908
Pack             00:00:17.2535228   (Exception of type 'Fake.DotNet.MSBuildException' was thrown.)
All              00:00:00           (skipped)
Total:           00:03:46.6883141
Status:          Failure
---------------------------------------------------------------------
Script reported an error:
-> BuildFailedException: Target 'Pack' failed.
-> One or more errors occurred. (Exception of type 'Fake.DotNet.MSBuildException' was thrown.)
-> MSBuildException: Exception of type 'Fake.DotNet.MSBuildException' was thrown.
Hint: To further diagnose the problem you can run fake in verbose mode `fake -v run ...` or set the 'FAKE_DETAILED_ERRORS' environment variable to 'true'
Hint: Could not find a version in your paket.dependencies file, consider adding 'version 5.257.0' at the top of your dependencies file (/Users/runner/work/fantomless/fantomless/paket.dependencies).
Read fsprojects/FAKE#2193 for details.
knocte added a commit to nblockchain/fantomless that referenced this pull request Sep 20, 2021
More info: fsprojects/Paket#4069

It was giving this error:
Microsoft (R) Build Engine version 16.11.0+0538acc04 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

/Users/runner/.dotnet/sdk/5.0.400/MSBuild.dll -maxcpucount -property:Configuration=Release -property:PackageOutputPath=/Users/runner/work/fantomless/fantomless/bin /p:Title=Fantomas /p:PackageVersion=4.5.1 /p:Authors=Florian Verdonck Jindřich Ivánek /p:Owners=Anh-Dung Phan /p:PackageRequireLicenseAcceptance=false /p:Description=This library aims at formatting F# source files based on a given configuration.
Fantomas will ensure correct indentation and consistent spacing between elements in the source files.
Some common use cases include
(1) Reformatting a code base to conform a universal page width
(2) Converting legacy code from verbose syntax to light syntax
(3) Formatting auto-generated F# signatures. /p:Summary=Source code formatter for F# /p:PackageReleaseNotes=Fix StackOverflow exceptions when collecting ColMultilineItem list. [fsprojects#1839](fsprojects#1839)
Honor .fantomasignore file when processing a folder. [fsprojects#1834](fsprojects#1834)
Fix Overly indented members on a record type with accessibility modifier. [fsprojects#1824](fsprojects#1824)
Fix MultiLineLambdaClosingNewline not respected with function keyword. [fsprojects#1823](fsprojects#1823)
Fix Comment is lost at the end of a match. [fsprojects#1822](fsprojects#1822) /p:Copyright=Copyright © 2021 /p:PackageTags=F# fsharp formatting beautifier indentation indenter /p:PackageProjectUrl=https://github.com/fsprojects/fantomas -target:pack -verbosity:m /bl:/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/tmpZzrMRy.tmp.binlog /clp:ForceConsoleColor src/Fantomas/Fantomas.fsproj
  Fantomas -> /Users/runner/work/fantomless/fantomless/src/Fantomas/bin/Release/netstandard2.0/Fantomas.dll
  Paket version 6.0.0-rc006
  Total time taken: 0 milliseconds
/Users/runner/work/fantomless/fantomless/.paket/Paket.Restore.targets(370,15): error MSB4030: " " is an invalid value for the "NoDefaultExcludes" parameter of the "PackTask" task. The "NoDefaultExcludes" parameter is of type "System.Boolean". [/Users/runner/work/fantomless/fantomless/src/Fantomas/Fantomas.fsproj]
: /Users/runner/work/fantomless/fantomless/.paket/Paket.Restore.targets(370,15): error MSB4030: " " is an invalid value for the "NoDefaultExcludes" parameter of the "PackTask" task. The "NoDefaultExcludes" parameter is of type "System.Boolean".
Finished (Failed) 'DotNet:pack' in 00:00:16.4637972
Finished (Failed) 'Pack' in 00:00:16.4690846
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants