Skip to content
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

Add formatting #13

Merged
merged 6 commits into from
Sep 6, 2022
Merged

Add formatting #13

merged 6 commits into from
Sep 6, 2022

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented Sep 5, 2022

I'd like to start contributing to this repository.
It seemed like a good start to enable formatting.
I've added a --check to ensure everybody does it.

If you accept this PR, don't squash the commits otherwise the git-blame-ignore-revs-file won't be effective anymore.

Copy link
Owner

@safesparrow safesparrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing!

The change looks good, I look forward to not having to think how to format the code.

Question: Does the .editorconfig provided mean that Rider's auto-format will obey exactly the same rules as Fantomas CLI?

Command : string
Args : string
}
type CodebasePrepStep = { Command : string ; Args : string }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that this is the G-Research convention. Does it always try to fit the record definition in one line if it's short-enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could put fsharp_max_record_width = 0 in your .editorconfig.
This will use the long format if the length of the record is more than 0 characters.

@safesparrow
Copy link
Owner

safesparrow commented Sep 5, 2022

I ran Rider's auto-format on one of the files (Runner/Runner.fs) and it resulted in quite a few changes.
Here is my (default) formatting config:
image
Fantomas version is the one pulled in via dotnet tool:
image

Is it possible to fully align Rider? Do we need to extend .editorconfig with some extra options?

@nojaf
Copy link
Contributor Author

nojaf commented Sep 6, 2022

Yes, this is that paint point I mentioned when using Rider.
They pass their own default values for Fantomas when you don't have them in your .editorconfig.

The command line check doesn't know about these and you get mixed results.
The full set of default values for Fantomas would be:

indent_size=4
max_line_length=120
end_of_line=crlf
insert_final_newline=true
fsharp_space_before_parameter=true
fsharp_space_before_lowercase_invocation=true
fsharp_space_before_uppercase_invocation=false
fsharp_space_before_class_constructor=false
fsharp_space_before_member=false
fsharp_space_before_colon=false
fsharp_space_after_comma=true
fsharp_space_before_semicolon=false
fsharp_space_after_semicolon=true
fsharp_space_around_delimiter=true
fsharp_max_if_then_short_width=0
fsharp_max_if_then_else_short_width=60
fsharp_max_infix_operator_expression=80
fsharp_max_record_width=40
fsharp_max_record_number_of_items=1
fsharp_record_multiline_formatter=character_width
fsharp_max_array_or_list_width=80
fsharp_max_array_or_list_number_of_items=1
fsharp_array_or_list_multiline_formatter=character_width
fsharp_max_value_binding_width=80
fsharp_max_function_binding_width=40
fsharp_max_dot_get_expression_width=80
fsharp_multiline_block_brackets_on_same_column=false
fsharp_newline_between_type_definition_and_members=true
fsharp_align_function_signature_to_indentation=false
fsharp_alternative_long_member_definitions=false
fsharp_multi_line_lambda_closing_newline=false
fsharp_experimental_keep_indent_in_branch=false
fsharp_blank_lines_around_nested_multiline_expressions=true
fsharp_bar_before_discriminated_union_declaration=false
fsharp_experimental_stroustrup_style=false
fsharp_keep_max_number_of_blank_lines=100
fsharp_strict_mode=false

Even though the command line doesn't need any of those, to get the same result out of Rider you would need to add some of these.

@safesparrow safesparrow merged commit e572a2c into safesparrow:main Sep 6, 2022
@safesparrow
Copy link
Owner

Thanks @nojaf 👍

@nojaf nojaf deleted the add-formatting branch September 6, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants