cheshirekow / cmake_format

Source code formatter for cmake listfiles.
GNU General Public License v3.0
954 stars 105 forks source link

strange formatting for set_target_properties #217

Closed derived-coder closed 4 years ago

derived-coder commented 4 years ago

version 0.6.11 I get:

 set_target_properties(
     my_library
     PROPERTIES CXX_STANDARD 17 CXX_VISIBILITY_PRESET hidden CMAKE_CXX_STANDARD_REQUIRED ON
                CMAKE_CXX_EXTENSIONS OFF VISIBILITY_INLINES_HIDDEN 1
 )

I want:

set_target_properties(
    my_library
    PROPERTIES CXX_STANDARD 17
               CXX_VISIBILITY_PRESET hidden
               CMAKE_CXX_STANDARD_REQUIRED ON
               CMAKE_CXX_EXTENSIONS OFF
               VISIBILITY_INLINES_HIDDEN 1
)

How can I format it, via cmake-format like this?

cheshirekow commented 4 years ago

Note to self: TupleGroupNode derives from PositionalGroupNode instead of ArgGroupNode:

diff --git a/cmake_format/parse/additional_nodes.py b/cmake_format/parse/additional_nodes.py
index f7b359e3..f05ba16d 100644
--- a/cmake_format/parse/additional_nodes.py
+++ b/cmake_format/parse/additional_nodes.py
@@ -13,15 +13,15 @@ from cmake_format.parse.common import (
 )
 from cmake_format.parse.simple_nodes import CommentNode, OnOffNode
 from cmake_format.parse.argument_nodes import (
-    PositionalGroupNode, PositionalParser, PositionalSpec, StandardArgTree
-)
+    ArgGroupNode, PositionalGroupNode, PositionalParser, PositionalSpec,
+    StandardArgTree)

 logger = logging.getLogger(__name__)

 # pylint: disable=W0221

-class TupleGroupNode(PositionalGroupNode):
+class TupleGroupNode(ArgGroupNode):
   """A positinal argument group where each argument is a tuple of tokens."""

   @classmethod
derived-coder commented 3 years ago

is the fix available in the latest release? I cannot see it from the MR itself.

derived-coder commented 3 years ago

@cheshirekow I checked it, this works now. However I noticed following new issue: I want:

target_include_directories(
    MyLib
    PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/../../>
           $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../../>
           $<INSTALL_INTERFACE:include>
)

Instead, I get following:

target_include_directories(
    MyLib
    PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/../../>
           $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../../> $<INSTALL_INTERFACE:include>
)
Maverobot commented 3 years ago

Besides, instead of getting

set_target_properties(
  my_target
  PROPERTIES IMPORTED_LOCATION ${some_dir}/my_target.so
             IMPORTED_NO_SONAME TRUE)

I got

set_target_properties(
  my_target
  PROPERTIES IMPORTED_LOCATION ${some_dir}/my_target.so

             IMPORTED_NO_SONAME TRUE)

(There is a strange empty line added.)

I am using cmake-format 0.6.5


Using the following .cmake-format fixes the issue. However, shouldn't it work out of the box? :)

# -*- mode:yaml; -*-

# Specify structure for custom cmake functions
additional_commands:
  set_target_properties:
    kwargs:
      PROPERTIES:
        kwargs:
          IMPORTED_LOCATION: '*'
          IMPORTED_NO_SONAME: '*'
cheshirekow commented 3 years ago

cmake-format version 0.6.13 yields:

set_target_properties(
  my_target PROPERTIES IMPORTED_LOCATION ${some_dir}/my_target.so
                       IMPORTED_NO_SONAME TRUE)

with default config.

Maverobot commented 3 years ago

@cheshirekow Thanks. I think it is the time to use the lastest version :100: