-
Notifications
You must be signed in to change notification settings - Fork 35
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
GH Action: install Julia formatter action #81
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #81 +/- ##
=======================================
Coverage 84.75% 84.75%
=======================================
Files 5 5
Lines 223 223
=======================================
Hits 189 189
Misses 34 34 Continue to review full report at Codecov.
|
@barucden could you try out the style configure? |
Sure. I tried running the commands locally, and one thing "bothers" me. It seems that the formater always splits the arguments of an invoked function into multiple lines. For example - return ccall((:svm_train, libsvm), Ptr{SVMModel},
- (Ref{SVMProblem}, Ref{SVMParameter}),
- problem, param)
+ return ccall(
+ (:svm_train, libsvm),
+ Ptr{SVMModel},
+ (Ref{SVMProblem}, Ref{SVMParameter}),
+ problem,
+ param,
+ ) Perhaps it is not always necessary? Also notice the comma after the last argument. Is it possible to allow only selected format adjustments? E.g., to adjust only spaces around |
ATM, seems there isn't a config of enabling single rule only. I'm going the try out CustomStyle (https://domluna.github.io/JuliaFormatter.jl/dev/custom_styles/), |
I tried the YAS style. It does not split the argument list into multiple lines, which is nice. However, it is still difficult to set the appropriate row width. I personally try to keep each row 80 letters wide at most, except for the cases when I don't :) Sometimes I break this rule because it is just easier to read. I might be too conservative, but enforcing strictly defined formatting rules is perhaps unnecessarily inflexible. How about just publishing a "style guide"? The YAS style, for instance, looks quite alright to me. The development here is not that wild after all :) |
hmm, seems it's the more suitable way at this moment. Let's pick YAS as the guideline for this repo. 👍 And yes, it's just a guideline, the rules can be broke if more readable. Actually, the goal of formatter is to reduce the time spent on maintaining styles. I like your idea about enabling the only selected syntax transformation. I will try to work on that later if I have enough time bandwidth. |
Yeah, if it could be configured to just fix white-spaces, that would be great! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a good idea to me.
No description provided.