Skip to content

style: try make code more consistent and align HTML DSL to 4 spaces indentation #948

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 3, 2025
Merged

style: try make code more consistent and align HTML DSL to 4 spaces indentation #948

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8326dd5
style: try make code more consistent and align HTML DSL to 4 spaces i…
MangelMaxime Dec 30, 2024
af6c99a
style: remove style rules which have no effect for the GenerateHtml.f…
MangelMaxime Jan 2, 2025
68a33d0
style: adapt the configuration accordingly to @nojaf feedback
MangelMaxime Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions .editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ root = true

[*]
end_of_line = lf

Copy link
Collaborator

@nojaf nojaf Jan 2, 2025

Choose a reason for hiding this comment

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

Please don't remove [*.{fs,fsi,fsx}], honestly rule of thumb change as little as possible.

Copy link
Contributor Author

@MangelMaxime MangelMaxime Jan 2, 2025

Choose a reason for hiding this comment

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

These settings make sense for all the files but ok

[*.{fs,fsi,fsx}]
charset = utf-8
end_of_line = lf
indent_style = space
indent_size = 4
trim_trailing_whitespace = true
Expand All @@ -16,5 +13,14 @@ fsharp_max_array_or_list_width=120
fsharp_max_infix_operator_expression=80
fsharp_max_value_binding_width=120

# Specific settings for "View/HTML" related files
# It makes the code more consistent in term of spacing and indentation
[GenerateHtml.fs]
fsharp_experimental_elmish = true
fsharp_experimental_elmish = true
fsharp_array_or_list_multiline_formatter = number_of_items
fsharp_max_array_or_list_number_of_items = 0
Copy link
Collaborator

@nojaf nojaf Jan 2, 2025

Choose a reason for hiding this comment

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

I don't really like this one, it makes too many single item list format in three lines. Makes it all rather excessive.

Copy link
Contributor Author

@MangelMaxime MangelMaxime Jan 2, 2025

Choose a reason for hiding this comment

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

This makes the code more consistent it always align to the 4 spaces guides and avoid having to reformat the list over and over when adding a new element:

CleanShot 2025-01-02 at 11 10 52@2x

For example, if I want to add a new element to embed with that configuration I can simply press Enter and be ready with it. Without I, need to move the list, align it, split the inner value on multiple lines, etc. Or add ; to add the new item but if it is too long I need to go back manually placing stuff around.

Copy link
Collaborator

@nojaf nojaf Jan 2, 2025

Choose a reason for hiding this comment

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

pressing ; and formatting isn't the end of the world.
How many times are really facing this issue? If you had this once I really don't feel like this is worth it. If you have this 20 times in an hour I could kinda get it, but even then...

Copy link
Contributor Author

@MangelMaxime MangelMaxime Jan 2, 2025

Choose a reason for hiding this comment

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

More often than I want to have to deal with it 🙂

Copy link
Contributor Author

@MangelMaxime MangelMaxime Jan 2, 2025

Choose a reason for hiding this comment

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

Removing this code also make it harder to read some line IHMO.

CleanShot 2025-01-02 at 20 14 26@2x

I removed it but we can re-introduce if wanted before merging.

fsharp_multi_line_lambda_closing_newline = true
fsharp_multiline_bracket_style = aligned
fsharp_keep_max_number_of_blank_lines = 1
# Keep the if/then/else consistent
fsharp_max_if_then_else_short_width = 0
Loading
Loading