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.

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

fix: Reduce opinionation of editorconfig #234

Merged
merged 1 commit into from
Dec 2, 2024
Merged

fix: Reduce opinionation of editorconfig #234

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 16 additions & 11 deletions .editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ quote_style = double
# call_arg_parentheses = keep
# continuation_indent = 4
# max_line_length = 120
trailing_table_separator = never
trailing_table_separator = smart

## Whitespace options
# space_around_table_field_list = true
Expand All @@ -30,12 +30,15 @@ space_inside_square_brackets = false
space_around_table_append_operator = false
ignore_spaces_inside_function_call = false
# space_before_inline_comment = 1
# space_after_comment_dash = false

## Operator whitespace
# space_around_math_operator = true
# space_after_comma = true
# space_after_comma_in_for_statement = true
# space_around_concat_operator = true
# space_around_logical_operator = true
# space_around_assign_operator = true
Copy link
Contributor

@Sharparam Sharparam Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Commenting these out makes no sense?

Actually disregard since I misread these two diff lines entirely, they are just introducing the default values I missed. Teaches me to read code while sleep deprived.


## Alignment
# Many of these will be set to false since alignment does not play well with tab indentation
Expand All @@ -50,17 +53,19 @@ align_array_table = false
## Other indentation settings
never_indent_before_if_condition = false
never_indent_comment_on_if_branch = false
keep_indents_on_empty_lines = false
allow_non_indented_comments = true
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the usecase for non-indented comments?

Copy link
Collaborator Author

@wcjohnson wcjohnson Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No particular use case in mind except I see no reason why someone shouldn't be allowed to do it if they want to.

This stuff is all forced on anyone who has auto-formatting enabled, so I think we should only turn a setting on if we want to force all code to conform to it.

In other words, the question isn't "what's the use case" but rather "why is this so bad that no human being should ever be permitted to do it?"


## Line spacings
line_space_after_if_statement = max(1)
line_space_after_do_statement = max(1)
line_space_after_while_statement = max(1)
line_space_after_repeat_statement = max(1)
line_space_after_for_statement = max(1)
line_space_after_local_or_assign_statement = max(1)
line_space_after_function_statement = fixed(1)
line_space_after_expression_statement = max(1)
line_space_after_comment = max(1)
line_space_after_if_statement = max(2)
line_space_after_do_statement = max(2)
line_space_after_while_statement = max(2)
line_space_after_repeat_statement = max(2)
line_space_after_for_statement = max(2)
line_space_after_local_or_assign_statement = max(2)
line_space_after_function_statement = max(2)
line_space_after_expression_statement = keep
line_space_after_comment = keep
Copy link
Contributor

@Sharparam Sharparam Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Several of these don't make sense, but particularly the function spacing.

Edit: Actually there seems to be a bug with the formatter where it's subtracting 1 from every specified line spacing. So 2 actually means 1. Highly confusing but that means the values of 2 here are correct.

Copy link
Collaborator Author

@wcjohnson wcjohnson Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since code formatting is a matter of opinion, I don't think it's a question of whether it makes sense, which is more of a factual question, but rather whether it improves readability.

In this case, I was saving files under the original editorconfig and it was stripping out so much whitespace that all the code was running together as a single block, which for me makes it way harder to read.

I simply chose the less opinionated option here, so if someone really wants to format code in close blocks by hand they can, but at least the autoformatter won't force it on you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah the original values using 1 broke because the EmmyLua behaviour is unfortunately not intuitive, but is sadly considered not-a-bug (see my edit): CppCXY/EmmyLuaCodeStyle#191 (comment)

So most of it in the updated version is actually fine, but the keep values don't really make sense since that allows an infinite amount of empty lines to be added in some cases when it should be stripped down to 1, so everything should probably have max(2).


## Line breaks
break_all_list_when_line_exceed = true
Expand All @@ -69,4 +74,4 @@ break_all_list_when_line_exceed = true
## "Preferences"
ignore_space_after_colon = false
# remove_call_expression_list_finish_comma = false
end_statement_with_semicolon = replace_with_newline
end_statement_with_semicolon = keep
Copy link
Contributor

Choose a reason for hiding this comment

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

Semicolons at the end of lines is a legacy some C(++) programmers bring to Lua, and is not idiomatic, nor should it be allowed in clean Lua code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just chose the less opinionated option again. I think occasionally (rarely, i'll grant) it is useful to put two statements on the same line for readability or grouping reasons.

Not particular committed to this though, there is nothing i've seen in the code base where this is used/applicable so I don't mind changing it back.