search

fsharp / fslang-design

RFCs and docs related to the F# language design process, see https://github.com/fsharp/fslang-suggestions to submit ideas
518 stars 144 forks source link

[style-guide] Presence of shebangs in scripts #683

Closed mataha closed 1 year ago

mataha commented 2 years ago

Writing F# scripts is a great way of, among other things, creating project tooling to improve the development and build processes, by e.g. managing git hooks or setting up CI. Saying that, the preferable, cross-platform way of invoking a F# script is to prepend it with a shebang directive (in this case #!/usr/bin/env -S dotnet fsi) and make it executable in order to run it on Linux distros, macOS, WSL et al.

However, this is not mentioned at all in the de-facto F# code formatting guidelines and, as a consequence, formatters implementing the coding conventions of F# remove it entirely. I believe this should not be done for F# scripts (.fsx), while code and signature files should be fine as they currently are.

Using e.g. Fantomas, the following:

#!/usr/bin/env -S dotnet fsi --warn:5 --warnon:1182 --warnaserror+

open System
// Rest of the code...

Is rewritten as:

open System
// Rest of the code...

That should not be the case.

vzarytovskii commented 2 years ago

@nojaf fyi, might be interesting for you

nojaf commented 2 years ago

If the lexer/parser can capture this in the syntax tree, Fantomas can restore it.

dsyme commented 2 years ago

This can be captured, yes, we should treat this as an FCS/ fantomas bug

mataha commented 2 years ago

What's the next step then? Do I file a bug report in Fantomas, then try to fix it before v5 hits?

nojaf commented 2 years ago

The next step would be to raise a PR to dotnet/fsharp that adds the shebang as ParsedHashDirective (I think).

You can open an issue in Fantomas, but you would get the same reply there. This doesn't hinder the v5 release, we would accept a fix at any time for this.

nojaf commented 2 years ago

Actually, after taking another look at this, it can be solved quite easily on Fantomas' side without any compiler changes.

Looking at the AST, the shebang is parsed as a line comment. There is a bug in Fantomas that doesn't recognize this as a line comment on a single line because it does not start with //.

@mataha, please report an issue on Fantomas' side. You can do this using the online tool.

nojaf commented 1 year ago

@dsyme this was implemented in Fantomas a while ago, do we still need to do anything here?

cartermp commented 1 year ago

This can be closed as completed.