fortran-lang / fprettify

auto-formatter for modern fortran source code
https://pypi.python.org/pypi/fprettify
Other
370 stars 76 forks source link

Fixes and new options #99

Open Jellby opened 3 years ago

Jellby commented 3 years ago

A couple of fixes:

And new whitespace options:

pseewald commented 3 years ago

Thanks a lot!

Apparently initial indent (e.g. from fixed-format code) is removed if the first statement is program or module. Now it's also removed if it's subroutine or function. (Is an option needed for this behavor? What's the reason for this?)

This goes back to pr #53 which I made default later on. There is no option to disable this as I thought that program / module should have 0 indent in any case. fprettify only supports free format. I don't agree with removing indent for subroutine / function by default, fprettify can also be applied to parts of a file (e.g. when used within an editor) and there removing indent for subroutine / function is not the expected behaviour. Do you reallly need this? If yes, could you make it an option?

--disable-indent-first: disable indent of the topmost level (similar to --disable-indent-mod, but also for subroutines and functions)

Do you really need this? I don't think this is a reasonable convention but I'd accept this option if you really see a use case for it.

Do not crash when both --disable-indent and --disable-whitespace are passed

I was not aware of this, thanks for catching and fixing this. I'll extend tests to this case.

Thanks a lot for the more fine-grained whitespace options which are very welcome! Would you have time to also add unit tests for the new options (just add new methods test_*, see this example? I just realized that for some reasons the automatic testing on travis CI does not work at the moment, I'll have a look.

Jellby commented 3 years ago

fprettify only supports free format.

The point was first make a fixed-format file free-format compatible (https://software.intel.com/content/www/us/en/develop/documentation/fortran-compiler-oneapi-dev-guide-and-reference/top/language-reference/program-elements-and-source-forms/source-forms/source-code-useable-for-all-source-forms.html) manually or with some other tool, and then use fprettify to, among other things, remove the unnecessary 6-column indent.

I don't agree with removing indent for subroutine / function by default, fprettify can also be applied to parts of a file (e.g. when used within an editor) and there removing indent for subroutine / function is not the expected behaviour. Do you reallly need this? If yes, could you make it an option?

Oh, I didn't think of applying fprettify to a partial file, then I agree on not doing it by default. But I have plenty of files that contain only a single subroutine/function, and I want to force it to 0 indent. I'll add an option (--reset-indent as in #53, or --whole-file, or whatever).

--disable-indent-first: disable indent of the topmost level (similar to --disable-indent-mod, but also for subroutines and functions)

Do you really need this? I don't think this is a reasonable convention but I'd accept this option if you really see a use case for it.

"Need" is a strong word, but I do want it. As above, when a file contains only a subroutine or function, it feels like a waste to indent every line. Isn't that the same reasoning behind --disable-indent-mod?

pseewald commented 3 years ago

Oh, I didn't think of applying fprettify to a partial file, then I agree on not doing it by default. But I have plenty of files that contain only a single subroutine/function, and I want to force it to 0 indent. I'll add an option (--reset-indent as in #53, or --whole-file, or whatever).

Ok, then you can reintroduce the option --reset-indent or name it --reset-indent-subroutine, as you prefer.

"Need" is a strong word, but I do want it. As above, when a file contains only a subroutine or function, it feels like a waste to indent every line. Isn't that the same reasoning behind --disable-indent-mod?

Yes it is and please go ahead if this feature is useful to you.

Jellby commented 3 years ago

Anything else I should do?

foxtran commented 3 years ago

@pseewald, @Jellby, any updates? @Jellby, could you please provide tests?

Jellby commented 3 years ago

What do you mean? There are some new tests in fprettify/tests/__init__.py.

stigh commented 6 months ago

@gnikit, I think this PR looks good, and can be approved. I found --indent-select and disabling whitespace checks to be very useful.

Maybe this Do not crash when both --disable-indent and --disable-whitespace are passed could be improved, since it only avoids crashing, but still have no effect (ref https://github.com/pseewald/fprettify/issues/163). But I still suggest to merged it.