-
Notifications
You must be signed in to change notification settings - Fork 143
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
[style-guide] Formatting of argument lists #664
Comments
From the syntax tree view it's not possible to distinguish a namespace from a static member or a module member access. How it should probably be formulated is try to maintain line breaks inside expressions with dot access and only split them for parts that don't fit on line with current line length settings.
I'd say the same thing here: we should try to keep the original formatting choice for places where the style guide allows variations like this. |
Decision on (4) is here: #663 |
On (2) I agree, ideally we should never break namespaces, I think under any circumstances. However it will be difficult to implement because namespaces are not known to fantomas. I don't know how we could implement except a list of known namespaces. And really, good code should open the namespace. Overall I think we could say this fits under "bad code formats badly" |
On (3) I'm torn, it's not a simple choice. I'm aware it is a realy problem in practice. There is also a major risk of being inconsistent with functions and methods taking tupled paramters. The current guidance is here: https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-function-and-member-arguments We must also consider the variaitons. One indent (4-space) - current style guide
let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
...
let sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
) =
...
let sillyfuncWithParams
parameterName0
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
)
parameterName4 =
...
let rec sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
...
let rec sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
) =
...
let private sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
...
let private sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
) =
...
type C() =
static member sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
...
type C() =
static member sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
) =
... Two indent (8-space)
let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
...
let sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
) =
...
let sillyfuncWithParams
parameterName0
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
)
parameterName4 =
...
let rec sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
...
let rec sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
) =
...
let private sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
...
let private sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
) =
...
type C() =
static member sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
...
type C() =
static member sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3
) =
... |
@dsyme We've been using that double-indent approach in ReSharper.FSharp and I haven't seen cases where it wouldn't work well. It always makes it very clear where a function/method/type body begins. |
@auduchinok I've added tupled and mixed cases to the above. What do you think for static members? And is there any reason that case would be different? |
@auduchinok basically it seems it is only curried The question is would we use double-indentation for all of the above? Or just the |
I've repurposed this issue to be specifically about formatting of argument lists |
It could be something like that (i.e. follow the expression rules and keep the opening paren on the previous line): type C() =
static member sillyfuncWithParams(
parameterName1,
ignoredParameterName2,
ignoredParameterName3) =
... In our project I'd format it like this: type C() =
static member sillyfuncWithParams(parameterName1, ignoredParameterName2,
ignoredParameterName3) =
... or (to add a constructor example) type C(parameterName1, ignoredParameterName2,
ignoredParameterName3) =
static member sillyfuncWithParams(parameterName1, ignoredParameterName2,
ignoredParameterName3) =
... |
It's interesting to note that currently this mixed example: let sillyfuncWithParams
parameterName0
(parameterName1,
ignoredParameterName2,
ignoredParameterName3,
ignoredParameterName4,
ignoredParameterName5,
ignoredParameterName6)
=
1 formats the tuple portion differently to let sillyfuncWithParams
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3,
ignoredParameterName4,
ignoredParameterName5,
ignoredParameterName6
) =
1 @nojaf what's the currently implemented rule here? Is treating things taking one tuple of arguments as special? |
@dsyme I believe the mixed example never came up in the style guide before. |
Coming here from F# Slack after @nojaf request for feedbacks form the community. Personally, I agree that breaking namespace should be avoided and I like having each argument on their own lines when the formatting kicks in for multiline as it keeps the code consistent and I find it easier to read. I can scan vertically and don't have the make jumps with eyes to different indentation levels. What I mean by jump between different indentation levels is code like that, where you never know where you should move your eyes before hand. type C(parameterName1, ignoredParameterName2,
ignoredParameterName3) =
static member sillyfuncWithParams(parameterName1, ignoredParameterName2,
ignoredParameterName3) =
... Regarding, the silly function, I personally prefer this formatting because to me it is:
let sillyfuncWithParams
parameterName0
(
parameterName1,
ignoredParameterName2,
ignoredParameterName3,
ignoredParameterName4,
ignoredParameterName5,
ignoredParameterName6
)
=
1 |
I'm more specifically asking for feedback on It came up today that it could be beneficial to visually separate the parameters and the function body. So that the header of the function is more easily identified. Something perhaps like let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3
=
longBody |
In that case, I personally prefer to stick with one level of indentation at the time (so not making the parameters / function body) visually distinct from each other. This is because, I feel like when there will be nested functions, it could make the code heavier to read. To me having a two indentation level breaks the visual consistency, as now something the next scope is 1 level indentation or 2 level indentation and same for the previous scope. Example of nested functions: let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3
=
let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3
=
longBody
longBody |
I think it looks somewhat easier on the eyes if let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
longBody
...
longBody let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3 =
longBody
...
longBody
longBody
...
longBody |
Putting the comma at the beginning of the line for tupled arguments would make more sense. I would also like very much if the line length restriction would take in account the indent level of the construct as reference, rather than the first column. |
Are there many existing codebases that consistently indent wrapped parameters in function declarations by 8 spaces? I don't think I've seen that before. If not, it would seem like people haven't generally found the 8-space convention worth applying by hand (or that they find the 4-space convention more readable most of the time). I'd argue for keeping single indentation throughout. Using a single indentation for wrapped parameters after a I don't think I would normally double-indent function parameters in a declaration by hand.1 If the alignment of the parameters with the function name really looked odd or hard to read, I'd probably just avoid wrapping the parameters altogether, or rename or refactor things until wrapping was no longer necessary. Funny enough, ocamlformat's While I actually think doubling 2 to 4 looks fine, doubling 4 to 8 somehow feels heavy-handed to me. If double-indent here did make it into the style guide, would we also update the tooling everywhere to automatically double-indent after Footnotes
|
@Thorium for me, out of the blue, the first one, and I think it will be different, legitimately for anyone. Scanning for the single line with |
I guess that's another vote for "no" to automatic code formatting everywhere. |
@Thorium, it is a vote for a more fine tuned, no black / white, all / nothing approach. I'd like to have something that:
but does nothing drastic:
For example, if this was part of design ethos for the formatter, more than implementation details of AST, to only think of "code touch up tool" rather than "all in" it would change a lot my perception; I know none of this is trivial, in the core of the implementation, and the choices that at least, made fantomas possible, I'm grateful for. I don't think average F# code base that is not automatically formatted, nor due to lack of "advanced editor tooling vs other big names languages" is that unreadable either, that it warrants code formatting to be required, static analyzers that can be tailored per repo have more value to me, since the editors evolved a lot, so typing code is generally formatted well enough without a lot of care. I'm thinking conceptually, formatters could be more conservative things, and more clever (detect if some code is grid formatted, detect if indentation in file is all over the place, leave it intact, if it is is 90% one way, reformat code that is not commited), but I'm not putting it on fantomas, fantomas is awesome disregarding my views / desires, fantomas is solid and necessary for F# ecosystem. I faced couples of negative code reviews that were just on the surface of formatting, and carried almost 100% emotionally (this is bad for any team), or some frustration with "CI workflow in the way" using fantomas or stylecop enough to not be an "adopter by default", and seen codebases with no formatting and messed up indents everwhere, in very old languages, to appreciate more my time on the soundness of the code, and automatic formatting doesn't change this aspect as much as other aspects, even in crappiest formatted code (which gets formatted if it needs change, just campground cleaning). What I like though, is tools like editors putting braces, indenting for me, and if formatters can be tailored for "only pay for what you use", I'd adopt more of it, I still remember being amazed by emacs and stylish haskell when I was trying this at beginning of my journey with ML (by default, in records for example, it lines all the I think fantomas will make it there also, but the community needs to grow in size, and possibly more funding, or just more people that are of the "formatter or die" but needs more things than it offers, meaning more incentivized maintainers. For the discussion of argument list, why can't we:
so other contributors could come after and fill the gaps, as a clear process so everyone gets served, while consumers like G-Research and heavy fantomas users at least get the argument formatting processed in the most desired/desirable way? I trust all parties to not try to mess with F# :) Thanks for always offering psychologist consultation to me @Thorium :D |
exactly, I think fslang-design and styleguide should not be correlated 100% to what fantomas can do today, maybe it was necessary evil (such as fantomas dog fooding in compiler) and still has value to keep a correlation, but not tight coupling. I've contributed edits to styleguide, that keep things a bit more open-ended, than what fantomas will do, while keeping the defaults as main suggestion, and I don't think it is a loss for F# or fantomas. "automatic code formatting everywhere" sounds like dictature, help people writing / keeping pretty code, and not be emotional or stringent about formatting choices, is a better objective, for me. |
I think the limitation with Fantomas is that it takes apples, transforms them to bananas, then transforms bananas to apples again, claiming his new apples are better than the original ones. But any originality is lost in the process, the new apples are not the original ones. That's why it'll always be hard to transfer characteristics (or developer intention) through the process. But you can claim that software development shouldn't be artistic or fun, just all machine processed like your hamburgers. Which I'm ok with, I'm more scared of future merge-conflicts on long living branches. |
it is made by humans, for humans (primarily, AFAIK), and vast majority of it is not life or mission critical to warrant "enroll into army of no fun and everyone's ego thinks the same, hail Kim Jun Fantomas Syme!" at all stages, even if it is a lot of monopoly money in consideration or deciding what to use (as this doesn't equate life or mission critical defacto). Life is whatever people make it geared towards, and it sometimes involves automatized code formatting or emerging community consensus, pragmatic, without consideration if it makes F# "for artists" or for "purists mission critical, large team, boring only allowed" (if I had to make cliché of opposites on spectrum, spectrum which can't be annihilated ever, just expanded farther and non discriminatorily). The guidelines and fantomas brought a lot of clarity, in same fashion that "F# code I love" talk. I still offer psychologist consultation about bleak future of humans not able to learn due to AI being bad, to help people overcome such illusory anxiety :D |
just throwing it out there, I think we should discuss on those grounds, and possibly have this as metadata that is toolable upfront, rather than "html source of of fantomas doc is the source of truth" or "many places of fantomas codebase". Then the decision for first impl. and sole setting is implementor and sponsor driven, so long majority of community is not broadcasting alert. |
The 8-space is much better as it distinguishes arguments from the function itself and from the implementation. |
I'd like to suggest something in between: let sillyfuncWithParams
parameterName1
ignoredParameterName2
ignoredParameterName3
=
longBody
...
longBody e.g. parameters idented and |
In the current OSS projects. there exists a push of Fantomas to force implementing the coding conventions of F#, where as those coding-conventions are not complete, creating a gap which is not minded.
IMHO: The coding conventions should focus on maintainability. The intention of the original developer should come clearly visible from the code. Without any kind of statistical analysis, the computer doesn't see that intention from the source code, thus it should keep its fingers off and not auto-obfuscate the code to meet some artificial convention... anyway...
Many developers use wide-screen monitors to replace multiple monitor configuration.
So the coding conventions should adapt to this current world we live in, where the development-monitors being easily 300 characters in width, but still 50 in height.
Therefore, to try to adopt to the situation, I suggest that:
Arrays with data only should be allowed to write without line-breaks. Code-lines that will not be maintained line-by-line should be allowed to be long. Stroustrup lines fsprojects/fantomas#2277
Unnecessary breaking of namespaces into separate lines should be avoided. Don't break namespaces unnecessarily fsprojects/fantomas#2279
Functions with many parameters should add one tab more before function body Functions with many parameters should add one tab more before function body fsprojects/fantomas#2278
Mutation of an object should still be preferred as the only action on that line
Mutable expressions should be on one line fsprojects/fantomas#2280
The text was updated successfully, but these errors were encountered: