Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable clang format cli take2 #2818

Merged
merged 5 commits into from
Feb 27, 2024
Merged

Conversation

Lestropie
Copy link
Member

Replaces #2815.
Closes #2701.

While the content here did take a lot of effort to create, the resulting attribution of ~ 5,000 line additions & deletions was not commensurate with the volume of that effort. This replacement PR attributes the majority of the changes to MRtrixBot. It also explicitly separates myriad changes that I made to command interfaces that I discovered during the course of undoing clang-format's changes (& tweaking some of the code structure as I went), from the gross reversion of clang-format's changes.

Lestropie and others added 2 commits February 26, 2024 13:17
Refinements to command documentation as discovered during the generation of 0cec87a as part of #2815. Here the changes to the command documentation has been separated from the changes relating to the disabling of clang-format for CLI-relevant code.
When clang-format, introduced in #2652, applied its changes to MRtrix3 code relating to the command-line interface (particularly the way that options and their arguments are concatenated through the addition operator), the resulting code was often quite ugly. This commit replaces code relating to CLI configuration (both the usage() function in cmd/*.cpp, and in definitions of options and option groups utilised by multiple commands) with manually formatted code, and includes comment fields to disable further manipulation of that code by clang-format.
This commit is a refactor of 0cec87a generated as part of #2815, with the changes being to omit other modifications to the command documentation that are not directly related to clang-format (those changes are in the parent commit to this commit: 15e253c), and to change authorship of the commit.
@Lestropie Lestropie force-pushed the disable_clang_format_cli_take2 branch from e3c52e7 to 895628e Compare February 26, 2024 04:17
github-actions[bot]

This comment was marked as outdated.

github-actions[bot]

This comment was marked as outdated.

Manual correction of documentation generation for #2818, given issues encountered in #2822.
@Lestropie Lestropie self-assigned this Feb 27, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 41. Check the log or trigger a new build to see more.

" from the tracks passing through each voxel;"
" options are: " + join(voxel_statistics, ", ") +
" (default: mean)")
+ Argument ("type").type_choice(voxel_statistics)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

    + Argument ("type").type_choice(voxel_statistics)
                                    ^

" Valid choices are: "
"FACT, iFOD1, iFOD2, Nulldist1, Nulldist2, SD_Stream, Seedtest, Tensor_Det, Tensor_Prob"
" (default: iFOD2).")
+ Argument ("name").type_choice(algorithms)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

    + Argument ("name").type_choice(algorithms)
                                    ^

const OptionGroup OutputHeaderOption =
OptionGroup("Options for the header of the output image")
// clang-format off
const OptionGroup OutputHeaderOption = OptionGroup ("Options for the header of the output image")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'identifiers' [cppcoreguidelines-interfaces-global-init]

const OptionGroup OutputHeaderOption = OptionGroup ("Options for the header of the output image")
                  ^

" or comma-separated list of 3 voxel dimensions.")
+ Argument ("size").type_sequence_float()
+ Option ("datatype", "specify output image data type.")
+ Argument ("spec").type_choice(DataType::identifiers);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

    + Argument ("spec").type_choice(DataType::identifiers);
                                    ^

" when generating Apodised Point Spread Functions")
+ Argument ("lmax").type_integer (2, 20);

const OptionGroup TWIOption = OptionGroup ("Options for the TWI image contrast properties")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'contrasts' [cppcoreguidelines-interfaces-global-init]

const OptionGroup TWIOption = OptionGroup ("Options for the TWI image contrast properties")
                  ^

Argument("out", "the output warp image.").type_image_out();
+ Argument ("in", "the input warp image.").type_image_in()
+ Argument ("type", "the conversion type required;"
" valid choices are: " + join(conversion_types, ", ")).type_choice(conversion_types)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

                      " valid choices are: " + join(conversion_types, ", ")).type_choice(conversion_types)
                                                    ^

Argument("out", "the output warp image.").type_image_out();
+ Argument ("in", "the input warp image.").type_image_in()
+ Argument ("type", "the conversion type required;"
" valid choices are: " + join(conversion_types, ", ")).type_choice(conversion_types)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

                      " valid choices are: " + join(conversion_types, ", ")).type_choice(conversion_types)
                                                                                         ^

return OptionGroup("Data type options")
+ Option("datatype", "specify output image data type."
" Valid choices are: "
+ join(identifiers, ", ") + ".")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

                              + join(identifiers, ", ") + ".")
                                     ^

+ Option("datatype", "specify output image data type."
" Valid choices are: "
+ join(identifiers, ", ") + ".")
+ Argument("spec").type_choice(identifiers);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

          + Argument("spec").type_choice(identifiers);
                                         ^

Argument("spec").type_choice(error_types)

"specify nature of errors for shuffling;"
" options are: " + join(error_types, ",") +

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

               " options are: " + join(error_types, ",") +
                                       ^

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

"specify nature of errors for shuffling;"
" options are: " + join(error_types, ",") +
" (default: ee)")
+ Argument("spec").type_choice(error_types)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

        + Argument("spec").type_choice(error_types)
                                       ^

+ Option("output",
"output only the field specified."
" Multiple such options can be supplied if required."
" Choices are: " + join(field_choices, ", ") + "."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

             " Choices are: " + join(field_choices, ", ") + "."
                                     ^

" For complex data, min, max and std are calculated separately for real and imaginary parts,"
" std_rv is based on the real valued variance"
" (equals sqrt of sum of variances of imaginary and real parts).").allow_multiple()
+ Argument("field").type_choice(field_choices)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

      + Argument("field").type_choice(field_choices)
                                      ^

.allow_multiple() +
Argument("value").type_choice(tractogram_geometry_types)
"The geometry type to use when rendering tractograms"
" (options are: " + join(tractogram_geometry_types, ", ") + ")").allow_multiple()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

               " (options are: " + join(tractogram_geometry_types, ", ") + ")").allow_multiple()
                                        ^

Argument("value").type_choice(tractogram_geometry_types)
"The geometry type to use when rendering tractograms"
" (options are: " + join(tractogram_geometry_types, ", ") + ")").allow_multiple()
+ Argument("value").type_choice(tractogram_geometry_types)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

        + Argument("value").type_choice(tractogram_geometry_types)
                                        ^

" l2 (ordinary least squares);"
" lp (least powers: |x|^1.2);"
" Default: l2")
+ Argument("type").type_choice(linear_robust_estimator_choices)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

      + Argument("type").type_choice(linear_robust_estimator_choices)
                                     ^

" geometric (aligns geometric image centres);"
" none."
" (Default: mass)")
+ Argument("type").type_choice(initialisation_translation_choices)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

      + Argument("type").type_choice(initialisation_translation_choices)
                                     ^

" moments (rotation based on directions of intensity variance with respect to centre of mass);"
" none"
" (Default: none).") // TODO This can be combined with affine_init_translation.
+ Argument("type").type_choice(initialisation_rotation_choices)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

      + Argument("type").type_choice(initialisation_rotation_choices)
                                     ^

"Default: diff") +
Argument("type").type_choice(linear_metric_choices)
" Default: diff")
+ Argument("type").type_choice(linear_metric_choices)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

      + Argument("type").type_choice(linear_metric_choices)
                                     ^

" l2 (ordinary least squares);"
" lp (least powers: |x|^1.2);"
" Default: l2")
+ Argument("type").type_choice(linear_robust_estimator_choices)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

      + Argument("type").type_choice(linear_robust_estimator_choices)
                                     ^

@Lestropie Lestropie merged commit c76b256 into dev Feb 27, 2024
6 checks passed
@Lestropie Lestropie deleted the disable_clang_format_cli_take2 branch February 27, 2024 10:35
Lestropie added a commit that referenced this pull request Feb 28, 2024
- Some limited modification of code structure to be more faithful to C++ changes in #2818.
- Resolution of documentation changes occurring in #2678, particularly in its merge to dev following cmake adoption (#2689).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants