search

fsprojects / fantomas

FSharp source code formatter
https://fsprojects.github.io/fantomas
Other
758 stars 189 forks source link

Turn on strict indentation. #3014

Closed nojaf closed 6 months ago

nojaf commented 6 months ago

As expected, the strict indentation in the Fantomas.FCS parser was not enabled. I don't think it matters much for Fantomas, as we expect a valid tree anyway. This could be useful for the online tool though.

//cc @auduchinok

auduchinok commented 6 months ago

Thanks @nojaf!

I've also thought there's a chance we could want to have a setting for it somewhere in the online tool one day, but I don't think it's worth it much to have it in advance 🙂

auduchinok commented 6 months ago

I've thought a bit more and now I'm a bit worried that Fantomas doesn't know about the language version that is used in a project/SDK. It may start to parse things differently and produce wrong results on code that wasn't updated for the strict indentation and has indentation warnings. At the same time not having this setting enabled would work correctly for both updated (without these warnings) and not updated code. Maybe we should not turn it on in Fantomas yet... But I'd be happy use this mode in the online tool at least optionally.

auduchinok commented 6 months ago

I don't think it matters much for Fantomas, as we expect a valid tree anyway.

Without this setting some code would have warnings about the indentation, but not errors. Does it count as a valid tree?

I've just tried on example in the online tool, and it seems the strict indentation is already enabled.

nojaf commented 6 months ago

Interestingly enough, this change in the PR does not affect the parse error of

let x =
1

We need to pass in Some false in order to opt-out of strict indentation. So None and Some true seem to do the same thing. Which is not really what I expected.

I've thought a bit more and now I'm a bit worried that Fantomas doesn't know about the language version that is used in a project/SDK.

Nope, Fantomas assumes preview and doesn't do language versions. This is a very reasonable tradeoff in terms of maintenance.

auduchinok commented 6 months ago

We need to pass in Some false in order to opt-out of strict indentation. So None and Some true seem to do the same thing. Which is not really what I expected.

None just makes it use the defaults, and Some value allows you to override it with the value. With a newer compiler the defaults are to use the strict indentation. This setting is only expected to be used temporarily, we'll hopefully remove it in future.