Apply Runic.jl formatter #2070
Apply Runic.jl formatter #2070
Conversation
Review checklistThis checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging. Purpose and scope
Code quality
Documentation
Testing
Performance
Verification
Created with ❤️ by the Trixi.jl community. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2070 +/- ##
==========================================
- Coverage 96.32% 96.28% -0.05%
==========================================
Files 470 470
Lines 37447 37511 +64
==========================================
+ Hits 36070 36114 +44
- Misses 1377 1397 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Do you have an idea why CI fails? |
| @muladd begin | ||
| #! format: noindent |
There was a problem hiding this comment.
Sadly, all of our source files will be indented because of this...
There was a problem hiding this comment.
There seems to be no way around it. I tried
# runic: off
@muladd begin
# runic: onbut this just disables formatting for the whole file.
There was a problem hiding this comment.
There was a problem hiding this comment.
See the reply in the linked issue. We don't use this "wrapping full files in @muladd" thing in TrixiParticles.jl, so I don't really care about this indent.
That seems to be a bug in Runic.jl: julia> Runic.format_string("x, = (1, 2, 3)")
"x = (1, 2, 3)"That is, of course, not equivalent: julia> x, = (1, 2, 3); x
1
julia> x = (1, 2, 3); x
(1, 2, 3)Edit: fredrikekre/Runic.jl#58 |
|
Already fixed. |
|
Maybe you are aware but I want to point out https://gist.github.com/kateinoigakukun/b0bc920e587851bfffa98b9e279175f2 so you can keep the blame functionality working without everything pointing to this PR modifying it. |
| filename :: String | ||
| n_corners :: Int | ||
| n_surfaces :: Int # total number of surfaces | ||
| n_interfaces :: Int # number of interior surfaces | ||
| n_boundaries :: Int # number of surfaces on the physical boundary | ||
| n_elements :: Int | ||
| polydeg :: Int | ||
| corners :: Array{RealT, 2} # [ndims, n_corners] | ||
| neighbour_information :: Array{Int, 2} # [neighbour node/element/edge ids, n_surfaces] | ||
| boundary_names :: Array{Symbol, 2} # [local sides, n_elements] | ||
| periodicity :: Bool | ||
| element_node_ids :: Array{Int, 2} # [node ids, n_elements] | ||
| element_is_curved :: Vector{Bool} | ||
| surface_curves :: Array{CurvedSurfaceT, 2} # [local sides, n_elements] | ||
| current_filename :: String | ||
| unsaved_changes :: Bool # if true, the mesh will be saved for plotting |
There was a problem hiding this comment.
This alignment was a cool feature in JuliaFormatter.jl.
There was a problem hiding this comment.
Does it really increase the readability of the code though? (This) code isn't read column by column but row by row.
While creating Runic I have thought a bit about things like this. I also used to sometimes align things like thisbut I have realized that usually it is just to make me "feel better", it doesn't really increase the readability. When you stop worrying about things like this while writing the code (e.g. press space multiple times to align) and just accept the formatters rules it kind of gives a similar peace of mind. 🙂
There was a problem hiding this comment.
I think it does. I can clearly see which are scalar values and which are the big arrays that I might be looking for.
|
Before we discuss this newly formatted code to decide if we want to use Runic.jl, note the following:
I think it wouldn't be wise to use Runic.jl at this point, as we would end up with PR comments like "please split this long line", which were the main reason why we introduced automatic formatting in the first place. Once this feature exists, we can re-evaluate. |
CC @ranocha @sloede