trixi-framework / Trixi.jl

Trixi.jl: Adaptive high-order numerical simulations of conservation laws in Julia
https://trixi-framework.github.io/Trixi.jl
MIT License
522 stars 102 forks source link

Formatting is ugly sometimes #1758

Open efaulhaber opened 9 months ago

efaulhaber commented 9 months ago

This is ugly: https://github.com/trixi-framework/Trixi.jl/blob/d7e1f746462e806a317f005d4745c9c7db739d95/src/equations/equations.jl#L418-L424

This as well: https://github.com/trixi-framework/Trixi.jl/blob/d7e1f746462e806a317f005d4745c9c7db739d95/src/equations/compressible_navier_stokes_2d.jl#L133-L140

This could probably be fixed in JuliaFormatter.

efaulhaber commented 9 months ago

I didn't just create this issue for fun. Here is a fix for the second part: https://github.com/domluna/JuliaFormatter.jl/pull/785

This changes the second part to

- function gradient_variable_transformation(::CompressibleNavierStokesDiffusion2D{
-                                                                                 GradientVariablesPrimitive
-                                                                                 })
+ function gradient_variable_transformation(::CompressibleNavierStokesDiffusion2D{GradientVariablesPrimitive})
    cons2prim
end
- function gradient_variable_transformation(::CompressibleNavierStokesDiffusion2D{
-                                                                                 GradientVariablesEntropy
-                                                                                 })
+ function gradient_variable_transformation(::CompressibleNavierStokesDiffusion2D{GradientVariablesEntropy})

When applied to all of src and test, it looks like this: #1759.

ranocha commented 9 months ago

Nice, thanks! Is your PR to JuliaFormatter.jl already accepted and released?

efaulhaber commented 9 months ago

It's merged, but not yet released. But this only solves the second part. The first code snippet will still look ugly, so let's keep this issue open.

efaulhaber commented 9 months ago

Now it's released and #1759 is ready to merge.

efaulhaber commented 9 months ago

See my comment above. I only fixed the second problem.