mesa
mesa copied to clipboard
MESAHub
Fix detachment in implicit
allows b% other_implicit_function_to_solve to signal that the binary is detached, so b% check_implicit_rlo can take action accordingly
I don't want to pick on this PR as "wrong" (it's not!) but there's a general cosmetic point that I don't think we've discussed and possibly should.
The only actual change in binary/other/mod_other_implicit_rlo.f90 seems to be the addition of the detached logical argument. Pretty much the whole file is marked as changed, though, I think only because of the change to indentation. I don't think we have strict style here but it's hard to see what's actually changed when so much is just indents.
@matthiasfabry can you see if your editor can be configured to use https://editorconfig.org/ then you can point it at https://github.com/MESAHub/mesa/blob/main/.editorconfig which sets our indentation style. Or just make sure its set to 3 spaces per indentation.
@matthiasfabry can you see if your editor can be configured to use https://editorconfig.org/ then you can point it at https://github.com/MESAHub/mesa/blob/main/.editorconfig which sets our indentation style. Or just make sure its set to 3 spaces per indentation.
I suspect a lot of existing code doesn't adhere to this style, either... I personally favour just leaving things until a lot of code in a particular area is overhauled. I don't want to bike-shed over this but it does bury the meaningful changes in minor ones.
the diff view allows to hide whitespace changes, which clears up a lot! But yes, spacing should be addressed I believe. Eg. why do all module declarations start with 6 spaces? With the fortran standard of 80 characters per line, that's nearly 10% of your space gone. Nearly all of the spacing changes in files is my formatter removing those leading 6 spaces.
@rjfarmer I do use the .editorconfig, however it says nothing on indentation
the diff view allows to hide whitespace changes, which clears up a lot!
Aha! I wasn't aware of that option. Thanks!
the diff view allows to hide whitespace changes, which clears up a lot! But yes, spacing should be addressed I believe. Eg. why do all module declarations start with 6 spaces? With the fortran standard of 80 characters per line, that's nearly 10% of your space gone. Nearly all of the spacing changes in files is my formatter removing those leading 6 spaces.
Fixed form fortran requires things to start in column 6, though we do use free-form fortran we never really changed that style point.
I think it would be better for us to adopt a practice of avoiding commits with many lines of superficial whitespace changes. Can you configure your editor to not auto-format files when you're making small changes?
Of course. For this small a PR, I've reverted some commits.
Although I would like to see the dev team pushing to a more modern free-form Fortran indentation style. I'm sure there must be tools out there to reformat the whole .f90 codebase this way. (edit) As a matter of fact, It seems my IDE is able to do this.
Just a clarification, I meant that I locally reproduced the problem so it is real. Looking for a fix now.