Add option for formatting with spaces between function names and arguments #839
Conversation
alerque
force-pushed
the
space-in-function-defs
branch
from
94ba46a
to
530aa62
Compare
alerque
force-pushed
the
space-in-function-defs
branch
from
530aa62
to
cb823a1
Compare
|
I'm using this in production including setting the config option it introduces all over the place. The results are what I want to see. If the implementation here isn't the best way to do it or needs adjusting to be included I'm happy to adapt, but it would be nice if this could get upstreamed so I don't have to push by fork of StyLua on anybody contributing to my Lua projects ;-) |
|
I think I'm going to get some movement on #854 this weekend (which I want to merge before next release) so I also hope to review this too. Sorry for the wait! |
|
Sure fair enough. From an implementation standpoint there are probably better ways to do this (I was quite unfamiliar with the code base) even with the current parser, and improved full-moon interfaces may open up even better ways. From the perspective of the end user feature introduction the two important parts are the name of the new config option key and values paired with their respective expected test outputs. Everything in between is implementation details that could be refactored at any time. Personally I'm only interested in the |
There was a problem hiding this comment.
Implementation itself looks good to me, thank you!
the name of the new config option key and values paired with their respective expected test outputs
I'm trying to think if the key name is complete descriptive of what is actually happening here for someone with no context. The space isn't really after the function, so maybe it should be space_in_functions or space_in_functions_before_parentheses. Not sure though, naming is always hard.
| @@ -1158,7 +1165,8 @@ pub fn format_function_declaration( | |||
| shape | |||
| ) | |||
| .update_leading_trivia(FormatTriviaType::Append(leading_trivia)); | |||
| let formatted_function_name = format_function_name(ctx, function_declaration.name(), shape); | |||
| let formatted_function_name = format_function_name(ctx, function_declaration.name(), shape) | |||
| .update_trailing_trivia(FormatTriviaType::Append(function_definition_trivia)); | |||
|
|
|||
| let shape = shape + (9 + strip_trivia(&formatted_function_name).to_string().len()); // 9 = "function " | |||
There was a problem hiding this comment.
A point worth mentioning is that our "shape" calculation here (and potentially elsewhere) will not take into account the fact that a space was added after the function name (since we explicitly strip the trivia afterwards). Shape is used to store the amount of line budget used so far and how many characters we have left on a line. This opens up a very small edge case where the definition line now hits column_width + 1 characters because of the space, but shape only sees column_width and therefore we don't reformat correctly. It's quite a minor edge case though, unsure if it is worth rectifying until someone brings it up
| @@ -259,6 +259,7 @@ StyLua only offers the following options: | |||
| | `indent_width` | `4` | Character size of single indentation. If `indent_type` is set to `Tabs`, this option is used as a heuristic to determine column width only. | | |||
| | `quote_style` | `AutoPreferDouble` | Quote style for string literals. Possible options: `AutoPreferDouble`, `AutoPreferSingle`, `ForceDouble`, `ForceSingle`. `AutoPrefer` styles will prefer the specified quote style, but fall back to the alternative if it has fewer string escapes. `Force` styles always use the specified style regardless of escapes. | | |||
| | `call_parentheses` | `Always` | Whether parentheses should be applied on function calls with a single string/table argument. Possible options: `Always`, `NoSingleString`, `NoSingleTable`, `None`, `Input`. `Always` applies parentheses in all cases. `NoSingleString` omits parentheses on calls with a single string argument. Similarly, `NoSingleTable` omits parentheses on calls with a single table argument. `None` omits parentheses in both cases. Note: parentheses are still kept in situations where removal can lead to obscurity (e.g. `foo "bar".setup -> foo("bar").setup`, since the index is on the call result, not the string). `Input` removes all automation and preserves parentheses only if they were present in input code: consistency is not enforced. | | |||
| | `space_after_fuctions` | `Never` | Specify whether to add a space beetween the function name and parentheses. Possible options: `Never`, `Definitions`, `Calls`, or `Always` | | |||
There was a problem hiding this comment.
| | `space_after_fuctions` | `Never` | Specify whether to add a space beetween the function name and parentheses. Possible options: `Never`, `Definitions`, `Calls`, or `Always` | | |
| | `space_after_functions` | `Never` | Specify whether to add a space between the function name and parentheses. Possible options: `Never`, `Definitions`, `Calls`, or `Always` | |
| @@ -271,6 +272,7 @@ indent_width = 4 | |||
| quote_style = "AutoPreferDouble" | |||
| call_parentheses = "Always" | |||
| collapse_simple_statement = "Never" | |||
| space_after_fuctions = "Never" | |||
There was a problem hiding this comment.
| space_after_fuctions = "Never" | |
| space_after_functions = "Never" |
Closes #832
So far just some CLI options and tests. Work on implementation pending feedback in issue.All options implemented and tested.