ArgumentAwareCommandInvocationDumper is not aware

[adriaandegroot]

I was wondering why install() formats like this:

install(
    FILES
        ${CMAKE_CURRENT_BINARY_DIR}/CMConfig.cmake
        ${CMAKE_CURRENT_BINARY_DIR}/CMConfigVersion.cmake
    DESTINATION ${CMAKECONFIG_INSTALL_DIR}
)

(e.g. the multi-valued parameters for FILES are indented) but configure_package_config_file() formats like this:

configure_package_config_file(
    CMConfig.cmake.in
    ${CMAKE_CURRENT_BINARY_DIR}/CMConfig.cmake
    INSTALL_DESTINATION
    ${CMAKECONFIG_INSTALL_DIR}
    PATH_VARS
    CMAKECONFIG_INSTALL_DIR
    FIND_MODULES_INSTALL_DIR
    MODULES_INSTALL_DIR
)

(e.g. the multi-values parameters for PATH_VARS are not indented). I would expect long(-ish) lists of arguments for multi-values parameters to be indented. The seven_samurai() example looks a bit weird, too: why are not the parameters for SHICHIROJI indented? (Or maybe they need to be longer, so that multi-line layout might kick in).

[adriaandegroot]

While digging, I ended up looking in _split_by_keywords() in the ArgumentAwareCommandInvocationDumper class. It uses self.multi_value_keywords. Printing the value used there (I'm not a very sophisticated debugger-of-Python-programs) shows me a value that is not ["PATH_VARS"] which is what I would expect. Instead I get the (or a) base-class value: ['FILES', 'PERMISSIONS', 'CONFIGURATIONS'].

patch-gersemi.txt

I ended up with the attached patch; to me it looks like the attemped replacement of the base-class does not work as expected, and the wrong settings are used for most classes.

[adriaandegroot]

With this patch, the Seven Samurai example turns out like this (I added some extra so that the line length would be exceeded):

seven_samurai(
    three
    standalone
    arguments
    KAMBEI
    KATSUSHIRO
    GOROBEI foo
    HEIHACHI bar
    KYUZO baz
    SHICHIROJI
        foo
        bar
        baz
        /usr/local/share/cmake/
        ${CMAKE_CURRENT_BINARY_DIR}/CMConfig.cmake
    KIKUCHIYO bar baz foo
)

Which, to me at least, looks a lot nicer. (The example given for custom formatting only has a few short words, so it doesn't exceed the line-length settings; with the patch, gersemi still sticks to one line if it can)

[adriaandegroot]

I can PR this if you like, or you could just apply the patch from the comment above, or, since gersemi is opinionated and simple, you could say "no, it's supposed to be like that".

[BlankSpruce]

I'm going to assume some things along the way so please correct me if you do something differently.

Given that /tmp/foobar.cmake is (I've removed all the indentation on purpose):

configure_package_config_file(
CMConfig.cmake.in
${CMAKE_CURRENT_BINARY_DIR}/CMConfig.cmake
INSTALL_DESTINATION
${CMAKECONFIG_INSTALL_DIR}
PATH_VARS
CMAKECONFIG_INSTALL_DIR
FIND_MODULES_INSTALL_DIR
MODULES_INSTALL_DIR
)

I have this output:

$ gersemi --version
gersemi 0.7.4
lark 0.9.0
Python 3.9.7 (default, Oct 10 2021, 15:13:22) 
[GCC 11.1.0]
$ gersemi /tmp/foobar.cmake
configure_package_config_file(
    CMConfig.cmake.in
    ${CMAKE_CURRENT_BINARY_DIR}/CMConfig.cmake
    INSTALL_DESTINATION ${CMAKECONFIG_INSTALL_DIR}
    PATH_VARS
        CMAKECONFIG_INSTALL_DIR
        FIND_MODULES_INSTALL_DIR
        MODULES_INSTALL_DIR
)

In fact I see the same for all the 0.7.x versions. Would you be able to double check what's the difference between what I've written above and your setup?

Just to make things clear: list of things, when sufficiently long, is meant to be extra indented after PATH_VARS so if there's a bug I want to fix it.

[adriaandegroot]

I think I simplified it down too much. I have attached two example files.

• This one is ok: foobar-ok.txt there is a file() command, which ger formats onto one line. The configure_package_config_file() command after it is indented the way we both expect, and the way your example without file() shows.
• This one is not ok: foobar-bad.txt The file() command is broken into multiple lines (which kind of makes sense, there's a bunch of arguments to that command now), but then the configure_package_config_file() command that comes after it, is indented wrongly.

[BlankSpruce]

I can now confirm it's a bug. I'll try to fix it soon.

[BlankSpruce]

Version 0.7.5 with the fix has been released. Thanks for reporting this bug.

[adriaandegroot]

Very nice, thank you. My collection of CMake modules now features "use gersemi before submitting MRs" in the documentation, for consistency.