Allow expanding all short function definitions #861
Conversation
|
I generally like this since it's backwards compatible. The downside that comes to mind is that it opens up the door for doing this in all sorts of circumstances for other parts of the code. |
|
So what should I do:
|
There was a problem hiding this comment.
After a bit more thought I think what would make more sense to is to have this be a boolean option rather than something determined by line margin, after all, the use case you have requires it be set to 0. And from the API perspective a boolean it easier to reason about than an arbitrary number, especially when I don't think anyone is setting that number to anything that's not 0 or margin.
| @@ -1,7 +1,7 @@ | |||
| name = "JuliaFormatter" | |||
| uuid = "98e50ef6-434e-11e9-1051-2b60c6c9e899" | |||
| authors = ["Dominique Luna <[email protected]>"] | |||
| version = "1.0.59" | |||
There was a problem hiding this comment.
this could be 1.0.60 since it's not a breaking change of any kind
There was a problem hiding this comment.
I changed it. Although personally, I would interpret this PR as a feature that should increase the minor version instead of a bugfix that should increase the patch version.
|
Any suggestion for a name for the new boolean option? Right now we have: I assume we want to be backward compatible and not touch these but add another option that is false by default. |
|
i like |
|
@domluna can you approve CI? |
|
failure seems unrelated? maybe something to do with the GH action |
| function Options(args...) | ||
| opts = new(args...) | ||
| if (opts.force_long_function_def === true) && | ||
| (opts.short_to_long_function_def === false) | ||
| msg = """ | ||
| The combination `force_long_function_def = true` and `short_to_long_function_def = false` is invalid. | ||
| """ | ||
| throw(ArgumentError(msg)) | ||
| end | ||
| return opts | ||
| end |
There was a problem hiding this comment.
I think this function is causing failures on Julia < v1.6
There was a problem hiding this comment.
ERROR: LoadError: LoadError: syntax: ... is not supported inside "new"
There was a problem hiding this comment.
@jw3126 update this when you get the chance and it should be g2g
I would like to have a way to banish all short function definitions from a code base. This PR implements one way to make that possible, by setting the max length of short function definitions to zero. But I would be happy to implement a different mechanism if desired.
I will add docs once it is clear whether we want an option like this.