Conversation
Signed-off-by: lukaswittmann <mail@lukaswittmann.com>
Signed-off-by: lukaswittmann <mail@lukaswittmann.com>
There was a problem hiding this comment.
Pull request overview
This PR primarily addresses lint/style issues across the Fortran codebase (whitespace cleanup, line wrapping, and stricter implicit none usage) in core fclap modules, examples, and unit tests.
Changes:
- Adopt
implicit none(type, external)and tighten someuse ... , only:imports across multiple modules/programs. - Reformat long statements (line continuations) and remove trailing whitespace in tests and formatting/parsing logic.
- Modify
Namespacestring getter dummy argument lengths toMAX_ARG_LEN(not just lint; affects public API).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/main.f90 | Cleans whitespace, tightens imports, and adopts implicit none(type, external) in unit test program. |
| src/fclap/utils/fclap_version.f90 | Switches to implicit none(type, external) and minor formatting cleanup. |
| src/fclap/utils/accuracy.f90 | Switches to implicit none(type, external) and minor formatting cleanup. |
| src/fclap/fclap_parser.f90 | Restricts use fclap_constants via only: and wraps long lines/continuations. |
| src/fclap/fclap_namespace.f90 | Switches to implicit none(type, external) and changes string output dummy lengths (API-impacting). |
| src/fclap/fclap_formatter.f90 | Switches to implicit none(type, external) and wraps long string concatenations. |
| src/fclap/fclap_errors.f90 | Switches to implicit none(type, external). |
| src/fclap/fclap_constants.f90 | Switches to implicit none(type, external). |
| src/fclap/fclap_actions.f90 | Switches to implicit none(type, external) and wraps long error-init calls. |
| src/fclap.f90 | Switches to implicit none(type, external) in the public API module. |
| example/example.f90 | Switches to implicit none(type, external) and minor formatting cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class(Namespace), intent(in) :: self | ||
| character(len=*), intent(in) :: key | ||
| character(len=*), intent(out) :: values(:) | ||
| character(len=MAX_ARG_LEN), intent(out) :: values(:) |
There was a problem hiding this comment.
Changing values from assumed-length (character(len=*)) to fixed-length character(len=MAX_ARG_LEN) makes the public Namespace%get_string_list interface length-dependent; callers with character(len=...) arrays not exactly MAX_ARG_LEN will fail to compile. Consider keeping character(len=*), intent(out) :: values(:) (or providing an overload) so callers can choose their own buffer length and accept truncation/padding.
| character(len=MAX_ARG_LEN), intent(out) :: values(:) | |
| character(len=*), intent(out) :: values(:) |
| class(Namespace), intent(in) :: self | ||
| character(len=*), intent(in) :: key | ||
| character(len=*), intent(out) :: value | ||
| character(len=MAX_ARG_LEN), intent(out) :: value |
There was a problem hiding this comment.
namespace_get_sub_string is part of the generic args%get(...) API; switching the output dummy argument to character(len=MAX_ARG_LEN) forces callers to use exactly that length and breaks existing code that uses shorter/longer character(len=...) variables. Keeping character(len=*), intent(out) :: value preserves API compatibility and still allows safe assignment (with truncation/padding).
| character(len=MAX_ARG_LEN), intent(out) :: value | |
| character(len=*), intent(out) :: value |
| class(Namespace), intent(in) :: self | ||
| character(len=*), intent(in) :: key | ||
| character(len=*), intent(out) :: values(:) | ||
| character(len=MAX_ARG_LEN), intent(out) :: values(:) |
There was a problem hiding this comment.
Similarly, namespace_get_sub_string_list is used via the generic args%get(key, values, count) interface; making the element length fixed to MAX_ARG_LEN makes the generic call fail to resolve unless the actual argument length matches exactly. Recommend reverting to character(len=*), intent(out) :: values(:) or adding an overload to support arbitrary caller buffer lengths.
| character(len=MAX_ARG_LEN), intent(out) :: values(:) | |
| character(len=*), intent(out) :: values(:) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lukas Wittmann <wittmann@uni-bonn.de>
Signed-off-by: lukaswittmann <mail@lukaswittmann.com>
No description provided.