fsprojects / FSharp.Formatting

F# tools for generating documentation (Markdown processor and F# code formatter)
https://fsprojects.github.io/FSharp.Formatting/
Other
462 stars 155 forks source link

support `netstandard2.0` #890

Open smoothdeveloper opened 7 months ago

smoothdeveloper commented 7 months ago

I'm not sure if the choice for netstandard2.1 only was explicit or just happened.

I can't consume any assemblies from .net framework and I think it hurts people trying to transition from older releases of this repository (that were or may remain .net framework only).

Since the changes seem minor, I hope this can be considered, if this doesn't hold undergoing progress.

Note that due to Path.GetRelative and the workaround I have being not 100% ideal, I've set the project using it to still has both netstandard2.0 and netstandard2.1 (as in, it shouldn't impact consumers of new versions that were anyways consuming the netstandard2.1 assemblies.

The build of the repository is nice, I had to deal a bit with fantomas and I think we should improve the message when --check is infringing, I'll see if I can bring something about in fantomas itself.

smoothdeveloper commented 7 months ago

For context, I intend to consume this for https://ci.appveyor.com/project/smoothdeveloper/fsharp-data-sqlclient-an46a/builds/48667306.

If I put effort in upgrading, and given the state and constraints of the FSharp.Data.SqlClient repository, I need to be super conservative in adjusting each piece, I'm not super familiar with fsdocs and intend first to fix the branch where I move from SDK v2, the docs is the last step I need to fix for this and I'd prefer to do the minimum adjustments in order to merge the update to SDK 7, before I can. consider spending time on refreshing the docs for the project.

I also do intend to look at consuming this for .net framework project, right now I can't use the project for work solution due to netstandard2.1 only.

nhirschey commented 7 months ago

I’m not sure the reason for netstandard2.1. It seemed to happen in PR #621.

smoothdeveloper commented 7 months ago

Thanks @nhirschey, after a quick review, @dsyme there is putting considerations about FSharp.Formatting consumers, and more generally FCS and other things in F# tooling, with regard to not forcing everyone to update (to latest FSharp.Core, and then the rest).

So I'm hopeful this can be considered for inclusion, I can probably nudge / polish it more if we are concerned about something getting suddenly broken with this PR.

nhirschey commented 7 months ago

@smoothdeveloper, FSharp.Data is netstandard2.0 and it builds docs using the latest version of FSharp.Formatting.

Based on that I’m not sure this PR is necessary to upgrade FSharp.Data.SqlClient to modern docs.

nhirschey commented 7 months ago

@smoothdeveloper looks like you’re coming from scaffold, see here for possibly slightly out of date guidance: https://fsprojects.github.io/FSharp.Formatting/upgrade.html

smoothdeveloper commented 7 months ago

@nojaf, thanks.

I'll give a check to the analyzers, I also thought of two changes:

Your ongoing comments about Fantomas in multiple repositories are unnecessary and unproductive. Please discontinue this practice to maintain a focused environment.

Please give yourself some distance or ignore them if they affect you, they are both meant light heartedly in case people take me too seriously, and they must echo something I'm only the catalyst for, in some manner, at least give it some slack.

I'm personally looking for using it in places where I know it will help, but I likely need more knobs (reverse of .fantomasignore, etc.), and some of the views, however may clash with choices, just expand perspectives, if we aren't emotionally affected.

I also think framing formatting discussion with preparing list of options upfront, and which are those options in the samples given is a very productive thing, this came through my comments, we can take "only the good parts" and ignore the rest.

So sorry, but we have to tolerate me some more, I'll keep in mind you didn't like it and respect your feedback, which also must echo some from others.

In the past three years, there has been no similar request

I'd rather look at how many projects have to yet upgrade, and how easy it is for those, when done by maintainers (not by Don or you who must have done it a lot), I thought in my case, to complete my upgrade to sdk 7 and allow me to consume the bits in more general context, netstandard2.0 is good option, doesn't hurt.

Comments from Don in #621 also highlight we shouldn't upgrade depedencies at high pace for core projects, due to chain of dependencies.

I think this holds, even if most people just consume fsdocs as a tool.

Additional points:

@nhirschey, thanks, exactly as first thing, I'm upgrading from scaffold, and also need to check more this guide, and maybe things will be smooth for FSharp.Data.SqlClient.

You've probably seen I made similar changes in few repository, probably because new projects are initialized with netstandard2.1, but overall, it doesn't hurt but few cases, to support netstandard2.0 as lowest target, otherwise it cuts the ecosystem in two.

For FSharp.Data.SqlClient, it takes a lot of time to adjust things for the CI, and updating dotnet sdk, I wished to complete the migration without having to do the overhaul of the docs but still making one step to consume the fresh dependencies, so I'm closer to have the good bits running next time I put efforts.

I'll check more if I can consume netstandard2.1 assemblies for the upgrade or complete the guide.

Thanks for the reviews everyone!

nhirschey commented 7 months ago

Just realized I also maintain a netstandard2.0 library (FSharp.Data.Toolbox) that I converted from scaffold to modern fsdocs. Definitely doable.

Anyway, thinking back on my experience going from scaffold to fsdocs, I'd avoid trying to "upgrade" the scaffold docs build pipeline as much as possible. Just take your content from "docs/content" and follow the zero-to-hero guide for a clean docs build. I think starting over, porting only the content rather than your full docs build pipeline, will be easier.

smoothdeveloper commented 7 months ago

Thanks @nhirschey, I'm hopeful and will give it another shot.

Also, not a big deal if we can't get this PR merged, I understand why it could be even without knowing details, it was also a way to try out the repository for myself, it is great!

I think it is good to keep a door open to more direct usage of lower bits, AFAIU fsdocs tool isn't going to help if it is only entry point, for specific use case (snippets, integrate F# bits in other documentation system like sandcastle, and others), and I believe the community using the lower bits won't make a mess of this repository if we keep the door open, just expand the capabilities, more pretty printed (and formatted) F# everywhere 🙂.

smoothdeveloper commented 7 months ago

I adjusted for analyzers, although in case it is something specific to this repository or my own environment, the output contains this:

MSBuild version 17.8.3+195e7f5a3 for .NET unhandled expression at unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\fsdocs-tool\Program.fs:(10,4--16,9) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\StringParsing.fs:(325,12--325,24) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\Collections.fs:(58,8--64,23) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\Collections.fs:(58,8--64,23) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\StringParsing.fs:(325,12--325,24) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\Collections.fs:(58,8--64,23) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\StringParsing.fs:(325,12--325,24) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\Collections.fs:(58,8--64,23) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\StringParsing.fs:(325,12--325,24) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\StringParsing.fs:(325,12--325,24) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\Common\Collections.fs:(58,8--64,23) unhandled expression at C:\dev\src\github\fsprojects\FSharp.Formatting\src\FSharp.Formatting.Markdown\MarkdownParser.fs:(1170,4--1378,5)

I checked the code around those locations but didn't find anything suspect; along which I found this code and was wondering if it is tracked as fantomas issue or if we should try to minimize the reproduce for it and track it:

https://github.com/fsprojects/FSharp.Formatting/blame/6e1443941998de1f72d98c562624e3754f3dc6ae/src/FSharp.Formatting.Markdown/MarkdownParser.fs#L1153-L1159

As for this PR journey, if we don't find it appealing, I suggest to keep it open as a place where other people may discuss if they want netstandard2.0 support, and eventually close it when it is time to close "non merged / stalled PRs".

Since I'm not sure my feedback on fantomas is welcome, the question specific to this repository would be and how it is integrated:

can we run fantomas without --check if it is not a CI run?

Bonus is, can fantomas quote back the command line minus --check and say

you may run 'this' to fix the code formatting

(where this is whole quoted command without the --check)

If this is relevant to fantomas, and needed, I can bring an issue to fantomas repository.

smoothdeveloper commented 6 months ago

Note that there is a subtletly with how the FSharp.Formatting nuget package is made, in order for this to work properly I think I need to make the layout of the .nupkg remain the same, and the .fsx to work with either netstandard2.0 and netstandard2.1 paths.

I'll look a bit more if / how this can be done.

Turned into a draft for now.