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

Strings containing spaces at end of line change meaning #1941

Closed
1 of 3 tasks
dsyme opened this issue Oct 31, 2021 · 5 comments · Fixed by #2010
Closed
1 of 3 tasks

Strings containing spaces at end of line change meaning #1941

dsyme opened this issue Oct 31, 2021 · 5 comments · Fixed by #2010

Comments

@dsyme
Copy link
Contributor

dsyme commented Oct 31, 2021

If a multi-literal string contains a space at the end of a line then it changes semantic meaning when formatted.

This seems wrong, since Fantomas should never change meaning of source code, but it may be considered acceptable in the balance of things, especially if C# and other languages would trim spaces even in strings (as a general editor setting)

In the example below there are spaces at the end of the line within the string

Issue created from fantomas-online

Code

let s =
    """spaces-at-end-of-this-line   
bbb"""

Result

let s =
    """spaces-at-end-of-this-line
bbb"""

The value of s has changed

Problem description

Please describe here the Fantomas problem you encountered.
Check out our Contribution Guidelines.

Extra information

  • The formatted result breaks by code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.

Options

Fantomas 4.6 branch at 10/31/2021 14:52:39 - 420ef5c

Default Fantomas configuration

Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?

@dsyme
Copy link
Contributor Author

dsyme commented Oct 31, 2021

Note that as a workaround, normal (non-triple-quote) strings in F# can use "\032" as a space character

Triple quote strings cause more problems

This came up in this change in a test for FSharp.Formatting

https://github.com/fsprojects/FSharp.Formatting/pull/716/files#diff-3d4e1b6d62530b5574130d89d1068004861cb3525c7145eb9f4120426a5a3abbR168

@dsyme
Copy link
Contributor Author

dsyme commented Oct 31, 2021

I found another case where fantomas formatting causes an FSharp.Formatting test to fail for the same reason

https://github.com/fsprojects/FSharp.Formatting/blob/b667a4bfb682760d8b9023ef261f3217a01fb6ce/tests/FSharp.Literate.Tests/LiterateTests.fs#L882

Here a test is testing the exact output of formatting, and the test result expects to see a space at end of line on one line. Triple-quote strings are used because they're very convenient for such cases

@dsyme
Copy link
Contributor Author

dsyme commented Oct 31, 2021

(None of these recent bug reports are blocking, since I can adjust FSharp.Formatting either tests or behaviour, but they are interesting cases for fantomas)

dsyme added a commit to dsyme/FSharp.Formatting that referenced this issue Oct 31, 2021
dsyme added a commit to fsprojects/FSharp.Formatting that referenced this issue Nov 1, 2021
* apply fantomas formatting

* workaround formatting bugs

* address issues arising from fsprojects/fantomas#1941

* simplify code
@nojaf
Copy link
Contributor

nojaf commented Nov 1, 2021

Interesting, I believe there might be an issue when processing the F# string tokens.
image

The last token does not contain any content (only three spaces) so perhaps it is not picked up in TokenParser for this reason.

let stringContent =
let builder = StringBuilder()
stringTokens
|> List.fold
(fun (b: StringBuilder, currentLine) st ->
if currentLine <> st.LineNumber then
let delta = st.LineNumber - currentLine
[ 1 .. delta ]
|> List.iter (fun _ -> b.Append("\n") |> ignore)
b.Append(st.Content), st.LineNumber
else
b.Append(st.Content), st.LineNumber)
(builder, head.LineNumber)
|> fst
|> fun b -> b.ToString()

@nojaf
Copy link
Contributor

nojaf commented Nov 9, 2021

I found the culprit:

let internal dump (ctx: Context) =
let ctx = finalizeWriterModel ctx
ctx.WriterModel.Lines
|> List.rev
|> List.skipWhile ((=) "")
|> List.map (fun line -> line.TrimEnd())
|> String.concat ctx.Config.EndOfLine.NewLineString

We trim each line here.
This will be a bit tricky to fix as we lost all meaning of each line by that point.

Doing the trimming earlier in

let updateCmd cmd =
match cmd with
| WriteLine
| WriteLineBecauseOfTrivia -> doNewline m
| WriteLineInsideStringConst
| WriteLineInsideTrivia ->
{ m with
Lines = "" :: m.Lines
Column = 0 }
| Write s ->
{ m with
Lines = (List.head m.Lines + s) :: (List.tail m.Lines)
Column = m.Column + (String.length s) }

is also not that simple as you don't know which Write event will be the last one before a newline. Nor do we know that the Write is a part of writing string text.

@jindraivanek any thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants