[WIP] Removed trailing whitespace from all files referenced in the VS solution.

[NatElkins]

I used the VS find and replace functionality with the regex "[^\S\r\n]+(?=\r?$)" to remove trailing whitespace.

[msftclas]

Hi @NatElkins, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

[msftclas]

@NatElkins, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

[KevinRansom]

@NatElkins

I really like this PR ..because I am always messing with removing extra whitespace. However, it is going to mess with too many existing Branches and PR's and cause a ton of merge conflicts for our contributors.

Additionally it doesn't add mechanism to ensure that once we are whitespace clean it stays that way.

Perhaps a way forward would be to add a test that runs that ensures no trailing whitespace, and to enable it on a directory by directory basis. Then we could clean directories on a directory by directory basis and ensure that it stays clean.

What do you think?

Kevin

[NatElkins]

@KevinRansom Makes sense, I'll try and write something like that up.

[NatElkins]

@KevinRansom Where do you suppose the best place to put this is? I was thinking the easiest thing to do would be to write a PowerShell script similar to the following:

$fileMatches = @(
    '.\test.txt',
    '.\directory\*.fs'
)

$fileMatches | ForEach-Object {
    $matchedFiles = Get-ChildItem $_ -Recurse
    $matchedFiles | ForEach-Object {
        $fileName = $_.FullName
        $content = Get-Content $fileName
        $content | ForEach-Object {
            if ($_.Length -ne $_.TrimEnd().Length)
            {
                Write-Error "Trailing whitespace found in '$fileName'."
                exit 1
            }
        }
    }
 }

and put it in visualfsharp\tests.

Or would you prefer this as a test in a .fs or .fsx file?

[KevinRansom]

I think .fsx, for when we finally enable cross platform building.

Thanks

From: Nat Elkins [mailto:[email protected]]
Sent: Monday, August 22, 2016 3:30 PM
To: Microsoft/visualfsharp [email protected]
Cc: Kevin Ransom [email protected]; Mention [email protected]
Subject: Re: [Microsoft/visualfsharp] Removed trailing whitespace from all files referenced in the VS solution. (#1421)

@KevinRansomhttps://github.com/KevinRansom Where do you suppose the best place to put this is? I was thinking the easiest thing to do would be to write a PowerShell script similar to the following:

$fileMatches = @(

'.\test.txt',

'.\directory\*.fs'

)

$fileMatches | ForEach-Object {

$matchedFiles = Get-ChildItem $_ -Recurse

$matchedFiles | ForEach-Object {

    $fileName = $_.FullName

    $content = Get-Content $fileName

    $content | ForEach-Object {

        if ($_.Length -ne $_.TrimEnd().Length)

        {

            Write-Error "Trailing whitespace found in '$fileName'."

            exit 1

        }

    }

}

}

and put it in visualfsharp\tests.

Or would you prefer this as a test in a .fs or .fsx file?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/1421#issuecomment-241571189, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AE76FuFiFE8Aom_ek4k15R0PJp5k9EGXks5qiiLdgaJpZM4Jimct.

[PatrickMcDonald]

Isn't PowerShell cross-platform now? 😉

[forki]

Please please please no Powershell. We already have way too much weird
stuff in this repo.

Am 23.08.2016 9:16 vorm. schrieb "Patrick McDonald" <
[email protected]>:

Isn't PowerShell cross-platform now? 😉

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1421 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNKi7a2EFjMq1IV7cY98Cn34Lh9jXks5qip5MgaJpZM4Jimct
.

[KevinRansom]

Lol … I suppose it is but we do have a scripting language of our own ☺

From: Patrick McDonald [mailto:[email protected]]
Sent: Tuesday, August 23, 2016 12:17 AM
To: Microsoft/visualfsharp [email protected]
Cc: Kevin Ransom [email protected]; Mention [email protected]
Subject: Re: [Microsoft/visualfsharp] Removed trailing whitespace from all files referenced in the VS solution. (#1421)

Isn't PowerShell cross-platform now? 😉

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/1421#issuecomment-241647421, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AE76Fo_m4kPtmjNUMQgIipjG3BY1AHOeks5qip5PgaJpZM4Jimct.

[KevinRansom]

I have got your back Steffen.

From: Steffen Forkmann [mailto:[email protected]]
Sent: Tuesday, August 23, 2016 12:18 AM
To: Microsoft/visualfsharp [email protected]
Cc: Kevin Ransom [email protected]; Mention [email protected]
Subject: Re: [Microsoft/visualfsharp] Removed trailing whitespace from all files referenced in the VS solution. (#1421)

Please please please no Powershell. We already have way too much weird
stuff in this repo.

Am 23.08.2016 9:16 vorm. schrieb "Patrick McDonald" <
[email protected]:[email protected]>:

Isn't PowerShell cross-platform now? 😉

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1421 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNKi7a2EFjMq1IV7cY98Cn34Lh9jXks5qip5MgaJpZM4Jimct
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/1421#issuecomment-241647599, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AE76FgOAEoez88JV8fHejDlQS5J4SADbks5qip6NgaJpZM4Jimct.

[forki]

If we really plan to do it with a tool then we should ask the fsharplint/fantomas people. I mean these are the tools that are built for this purpose.

[KevinRansom]

If we are going to trim whitespace off of every file. I would like a mechanism to ensure that we keep it that way.

I have no preference as to how we go about ensuring it, beyond a preference not to add yet another scripting language to the repo.

Frankly though an fsx file that does a TrimEnd() and compares it with the original and fails for each line of every source file in the repo seems like sufficient tool to me, but then I have simple tastes. I'm sure that fantomas would also be able to deal with it too.

Kevin

[forki]

yes that's what I meant. a fantomas task that keeps it clean...

[NatElkins]

@KevinRansom
Sorry, kind of forgot about this. Anyway, I wrote the following:

open System.IO

let root = "C:\\Projects\\visualfsharp"

// Single * wildcards are permitted.  Where you might use ** to searh recursively,
// use SearchOption.AllDirectories.  To only search a single directory, use
// SearchOption.TopDirectoryOnly.
let matches =
    [|
        "src\*.fs", SearchOption.AllDirectories
        "tests\*.*", SearchOption.TopDirectoryOnly
    |]

let files =
    matches
    |> Seq.collect(fun (path, searchOption) ->
        seq { yield! Directory.EnumerateFiles(root, path, searchOption) })

let hasTrailingWhitespace fileName =
    let lines = File.ReadAllLines(fileName)
    lines
    |> Array.exists(fun line -> line.TrimEnd().Length <> line.Length)

let filesWithTrailingWhitespace =
    files
    |> Seq.fold(fun filesWithWhitespace file ->
       match hasTrailingWhitespace file with
       | true -> file::filesWithWhitespace
       | false -> filesWithWhitespace) []

filesWithTrailingWhitespace
|> List.iter(printfn "File with whitespace: %s")

Basically does what you're asking. Any feedback? Also, where in the test suite should it go? Doesn't seem to clearly fit anywhere that I can tell.

[KevinRansom]

@NatElkins

You can put the script here:
https://github.com/Microsoft/visualfsharp/tree/master/tests/scripts

Add a section to build.cmd that runs the script.

add a new option TEST_SOURCEFORMAT

run the test when build sourceformat is specified.

We can add that to the CI and it will fail whenever anyone adds whitespace. Or something like that. Then none need ever worry about it again :-)

Thanks for doing this:

Kevin

[NatElkins]

@KevinRansom All checks have passed, does this look okay?

[KevinRansom]

I will be taking a look at PR's over the weekend. Kind of stressing over portable pdbs right now.

[NatElkins]

No worries, just wanted to put it on your radar. Thanks for taking a look, and good luck with that other stuff!

[KevinRansom]

@NatElkins

Sorry mate, it seems like we are never going to get around to this.

Sorry.