Reformatting a list expression with if/else changes the list expression

gubser commented 11 months ago

Code

[
    if 1 = 1 then 1 else 2
    3
]

Result

[ if 1 = 1 then
      1
  else
      2
      3 ]

Problem description

Fantomas reformats the list in a way that produces a different list:

> [
-     if 1 = 1 then 1 else 2
-     3
- ];;
val it: int list = [1; 3]

> [ if 1 = 1 then 1 else 2; 3 ];;
val it: int list = [1]

Extra information

[x] The formatted result breaks my code.
[ ] The formatted result gives compiler warnings.
[ ] I or my company would be willing to help fix this.
[ ] I would like a release if this problem is solved.

Options

Fantomas main branch at 1/1/1990

Default Fantomas configuration

_{Did you know that you can ignore files when formatting by using a .fantomasignore file?} _{PS: It's unlikely that someone else will solve your specific issue, as it's something that you have a personal stake in.}

gubser commented 11 months ago

Hi there!

I've found a workaround by setting fsharp_array_or_list_multiline_formatter to number_of_items and have it set to 1. See fantomas-online

Another alternative is put the expression between brackets.

[
    (if 1 = 1 then 1 else 2)
    3
]

results in

[ (if 1 = 1 then 1 else 2); 3 ]

nojaf commented 11 months ago

Hello,

It seems like the AST has changed after formatting, leading to an invalid result. Maybe we can extend https://github.com/fsprojects/fantomas/blob/a49d1703900692c72ce5378c97316e2be4c01b83/src/Fantomas.Core/CodePrinter.fs#L1800-L1824

and not check for YieldExpr but always go multiline when there is any Expr.IfThenElse.

Are you interested in submitting a PR?

gubser commented 11 months ago

Cool! Yes, I'd like to submit one. But it's going to be next week.

gubser commented 10 months ago

TL;DR: I've given up setting up Fantomas dev env, sorry guys. I run into issues installing .NET 8 sdk on Ubuntu 22.04.3 and it seems complicated enough that I would easily spend the whole day trying to understand what's going on under the hood.

I'm having trouble installing .NET 8 deb packages (conflict) and using the .tar.gz packaged sdk doesn't work either.

In Getting Started it says to install Release/7.0.4xx. But global.json requires at least 8.0.100-preview.7.23376.3, so I went ahead and tried installing 8.0.

Then in dotnet-installer it says I need to install the deps separately from this page I assume. However, sudo dpkg -i dotnet-host-x64.deb fails with

dpkg: regarding dotnet-host-x64.deb containing dotnet-host:
 dotnet-host-7.0 conflicts with dotnet-host
  dotnet-host (version 8.0.0-1) is to be installed.

But I'm not gonna uninstall dotnet-host-7.0 because I need it for work.

I tried another approach by downloading the sdk as a .tar.gz from this page and 'installing' according to instructions (extract, set DOTNET_ROOT and PATH variable). With that approach I got further, I could do dotnet tool restore and dotnet restore. But when I ran dotnet fsi build.fsx it failed in CheckFormat stage on dotnet fantomas src docs build.fsx --check because it didn't find the fantomas tool. I temporarily removed that one just to test if the rest would succeed. But then the script failed in stage UnitTests because it could not find my .NET 7.0. So apparently, it also needs .NET 7 as well.

Maybe I'll try again some later time when os-specific .NET 8 packages are out.

nojaf commented 10 months ago

Hi there, sorry to hear you had so many issues installing dotnet. Our Getting Started guide should indeed refer to the latest SDK in the global.json. So, you were right to go with dotnet 8 rc 2. It is very sad to hear that just installing things was so painful.

It also sounds like you hit https://github.com/dotnet/sdk/issues/35989. Which is another frustrating thing for sure.

I typically follow the steps listed here when I need to install the SDK on Linux.

# Download the SDK
curl -O -J https://download.visualstudio.microsoft.com/download/pr/9144f37e-b370-41ee-a86f-2d2a69251652/bc1d544112ec134184a5aec7f7a1eaf9/dotnet-sdk-8.0.100-rc.2.23502.2-linux-x64.tar.gz
DOTNET_FILE=dotnet-sdk-8.0.100-rc.2.23502.2-linux-x64.tar.gz
export DOTNET_ROOT=$HOME/.dotnet 
mkdir -p "$DOTNET_ROOT" && tar zxf "$DOTNET_FILE" -C "$DOTNET_ROOT" 
export PATH=$PATH:$DOTNET_ROOT:$DOTNET_ROOT/tools

I'm not quite sure why the Unit tests failed. This may need another rollforward hack.

I appreciate you taking the time to look into this and feel bad for the puzzling state of dotnet. Hopefully the situation will improve in the future.

dawedawe commented 10 months ago

@gubser So, you tried to mix distro packages and tar balls? IIRC that's very likely to fail as you also experienced.

fsprojects / fantomas