dangle_parens is not working as intended. about cmake_format HOT 7 CLOSED

I don't know if it's up to some vote, but I personally prefer option 1.

from cmake_format.

Uh oh. It could be a regression. Was it previously working differently? Can you post your failing listfile and config?

from cmake_format.

My command:
cmake-format --config-file ${CONFIG_PATH}/cmake_config.yaml ./CMakeLists.txt
I am very confident it is finding the cmake_config.yaml as other changes are reflected.
I just started using the tool so I do not know how it used to work. Unless I misunderstand what the setting does I think the paren after the last source file should be on it's own line.
InputCMakeLists.txt
Result.txt

Appended .txt to these so I could upload them:
cmake_config.py.txt
cmake_config.yaml.txt

from cmake_format.

Ok yeah, I see what's going on. Looking at the unit tests the logic is that dangle_parens is only applied if the command arguments don't fit on a column. Here's an illustration of intended behavior:

A:

      some_long_command_name(longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname)

versus
B:

      if(foo)
        some_long_command_name(
          longargname longargname longargname longargname longargname
        )
      endif()

Because A all fits in one column after the command name, the paren is not "dangled". Whereas B got wrapped to a new line underneath the command. If I were to turn this issue into a feature request, it sounds like what you're looking for would be an additional level of dangling parens i.e. "always". I considered implementing it that way to begin with but felt like the closing paren would be too easy to miss for a reader. Consider below two possible implementations of dangle_parens = 'always':

Option 1:

      some_long_command_name(longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname
      )

Option 2:

      some_long_command_name(longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname
                             longargname
                            )

To me, both of these have readability issues because it's hard for me to determine the "open" part of closing paren. If one of these is actually preferable to you, however, let me know which one you like and I can look into adding it as an option.

I will note that your example case is a bit different than the canonical one I designed for. In your case the command name is very short so dangling parens might be oK. For example:

Option 1:

      short(longargname
            longargname
            longargname
            longargname
            longargname
            longargname
            longargname
            longargname
       )

Option 2:

      short(longargname
            longargname
            longargname
            longargname
            longargname
            longargname
            longargname
            longargname
           )

Though my personal interpretation is that neither of these are as readable. Maybe really the option here should be "don't dangle parens for single-column wraps with command names longer then X characters".

from cmake_format.

"Always" dangling parens are desirable not so much for readability but for consistency so a user can comment out or remove any entry in that list without worrying they are removing the parentheses.

I would like the second option but I know some people/codebases I have seen would prefer your first option as well. Looking at clang-format configs I think there are many, many opinions on the best way to format anything.

I like your tool a lot and it might become THE Thing as Cmake takes over (which seems to be happening).
Perhaps long term a regex based custom config option would let people satisfy themselves with the tiny details(For example I also would like the ability to put spaces INSIDE brackets so set(a) becomes set( a ) ). A general config tool might take the pressure off you to make every little thing happen. Just a thought.

from cmake_format.

@andrewwaldrum I think that optimizing for write-ability is often the wrong thing. That said, I'm not opposed to adding some options to move the closing parens to a new line regardless of context. @sum01 noted. When implemented, I'll add options for either case.

from cmake_format.

You are awesome.

from cmake_format.