Libelektra: cmake formatting

[ingwinlu]

Running into an issue where cmake_format suggests the following changes:

 if (ADDTESTING_PHASE)
-   if (CMAKE_C_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-       # Disable test for Clang + valgrind combination
-       # https://github.com/ElektraInitiative/libelektra/issues/1871
+   if (CMAKE_C_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") # Disable test for Clang + valgrind combination
+                                               # https://github.com/ElektraInitiative/libelektr
+                                               # a/issues/1871

Can we configure it so it does not propose moving the comments next to the ifs? Seems way less readable to me.

[ingwinlu]

https://github.com/cheshirekow/cmake_format does not list an appropriate config, but in the example shown it also does not format that way.

[ingwinlu]

Alright that can be solved by adding an additional line between the if and the comment.

But what about this case now:

-       add_plugintest (lua
-           MEMLEAK
-           WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../../bindings/swig/lua/
-           INSTALL_TEST_DATA)
+       add_plugintest (lua MEMLEAK WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/../../bindings/swig/lua/ INSTALL_TEST_DATA)

I am quite surprised because I did not touch those lines at all.
Plus again putting everything in one line seems counter productive.

[sanssecours]

I am quite surprised because I did not touch those lines at all.

I think the issue is the indentation level. In the master branch cmake-format is not able to fit the statement into one line (indentation level 3). This is not the case in the branch jenkinsfile_misc anymore (indentation level 2).

[markus2330]

Is it also possible to add comments within function calls to avoid everything being squeezed in one line:

-add_plugin (blockresolver SOURCES blockresolver.h blockresolver.c LINK_ELEKTRA elektra-invoke ADD_TEST INSTALL_TEST_DATA TEST_README)
+add_plugin (blockresolver #
+           SOURCES blockresolver.h
+                   blockresolver.c #
+           LINK_ELEKTRA elektra-invoke #
+           ADD_TEST #
+           INSTALL_TEST_DATA #
+           TEST_README)

see 455bdcc7cac9b6696bfa38f1bceb4d5fbb7e870f

[sanssecours]

see 455bdcc

I do not like the idea of adding empty comments (at all). By the way, you only need to add a single comment:

add_plugin (blockresolver #
        SOURCES blockresolver.h
            blockresolver.c
        LINK_ELEKTRA elektra-invoke
        ADD_TEST
        INSTALL_TEST_DATA
        TEST_README)

.

[sanssecours]

If you do not like the formatted code produced by cmake-format you can always disable formatting for part of a file by adding the comment # cmake-format: off:

# cmake-format: off
add_plugin (blockresolver
        SOURCES blockresolver.h
            blockresolver.c
        LINK_ELEKTRA elektra-invoke
        ADD_TEST
        INSTALL_TEST_DATA
        TEST_README)

. I would use that directive very sparingly though. The whole idea of using an automated code formatter is (in my opinion) that developers do not argue about code formatting issues.

[markus2330]

Thank you for the hint, I updated it in d7f692d9477db82c3f02bb4dad17b62c38eb48a9

I also do not like the idea but is there any other way? I like unreadable code even less.

I added the purpose of the comment, it now looks similar to ("clang-format" instructions) and actually has a similar purpose.

What do you think? Can we add this guideline to CODING.md?

[markus2330]

Thanks for the tip, I did not know that there is a cmake-format: off.

But the directive cmake-format: off might be too heavy. As you said it easily leads to differently formatted code and discussions about it (we wanted to avoid).

What we do here is actually very similar to adding comments to enums as in clang-format, e.g., like done in 0d413ce6819dc8c07a8e7e588ff830dccdd166d2

[ingwinlu]

The code formatters i worked with before (mostly flake8) did not care if I did indent lines earlier than the maximum hence I did not expect the forced changes.

Maybe just reduce the max line length a bit till it fits our needs?
Or open an issue upstream asking for an option to 'overindent' i.e. indenting earlier than needed to be allowed)

[sanssecours]

I like unreadable code even less.

I do not see any unreadable code in that line at all. If you want more readable code we can always reduce the maximum line length (and get rid of tabs while we are at it).

I also do not like the idea but is there any other way?

See comment above.

What do you think?

My opinion has not changed in the last 20 minutes 😊.

What we do here is actually very similar to adding comments to enums as in clang-format, e.g., like done in 0d413ce.

I do not think that is the same. The comments in commit 0d413ce provide actual value (documentation), an empty comment does not.

[markus2330]

I do not see any unreadable code in that line at all. If you want more readable code we can always reduce the maximum line length (and get rid of tabs while we are at it).

Then let us better drop the discussion, not much to be gained from it.

I do not think that is the same. The comments in commit 0d413ce provide actual value (documentation), an empty comment does not.

You are right, I only did not find any useful comment to be written at this place. I only liked the previous formatting of add_plugin much more (nearly all plugins are affected). But as said, let us forget this. I am okay with both variants.

I like the suggestion of @ingwinlu but I think it is really hard to specify when this 'overindent' needs to be done. It has something to do with the length and count of multi-value keywords.