BlankSpruce / gersemi

A formatter to make your CMake code the real treasure
Mozilla Public License 2.0
156 stars 8 forks source link

Inconsistent formatting of set command with favour-inlining enabled #18

Closed PatTheMav closed 8 months ago

PatTheMav commented 8 months ago

With current master branch of gersemi I noticed that even with favour-inlining enabled some invocations of the set command get split up into multiple lines.

In my case the following invocation is kept on a single line:

set(CCACHE_SUPPORT ON CACHE BOOL "Enable ccache support")

Whereas the following invocation is split into multiple lines:

set(
  uuid_token_lengths
  8
  4
  4
  4
  12
)

This happens even though the column limit is set to 120, so the invocation should fit into a single line just fine.

BlankSpruce commented 8 months ago

This one is the funny one that perhaps I should revisit. In imaginary Python based system these two commands would be equivalent to:

set(
    name="CCACHE_SUPPORT", 
    value=["ON"], 
    cache="BOOL", 
    doc="Enable ccache support"
)

set(
    name="uuid_token_lengths",
    value=["8", "4", "4", "4", "12"],
    cache=None,
    doc=None
)

I've made this arbitrary choice initially that set with list of more than 4 arguments is transfomed to multi-line variant.

BlankSpruce commented 8 months ago

I think I will close this one with an update to README and help. While favour-inlining tries its best to inline the code I think it should be done within reason hence the "favour", not the "force". The threshold of 4 (arbitrary no doubt) for group of arguments without keyword (e.g. add_library, set, list(APPEND)) is a good enough heuristic for "this will explode into huge diff if we don't split into multiple lines early".

Examples where it applies:

set(FOO thing1 thing2 thing3 thing4)
set(FOO
    thing1
    thing2
    thing3
    thing4
    thing5
)
list(APPEND FOO thing1 thing2 thing3 thing4)
list(
    APPEND
    FOO
    thing1
    thing2
    thing3
    thing4
    thing5
)
add_library(FOO SHARED thing1 thing2 thing3 thing4)
add_library(
    FOO
    SHARED
    thing1
    thing2
    thing3
    thing4
    thing5
)

and where it doesn't:

target_link_libraries(FOO PUBLIC thing1 thing2 thing3 thing4)
target_link_libraries(
    FOO
    PUBLIC thing1 thing2 thing3 thing4 thing5
)

In hindsight I should have probably made favour-expansion (or some flavour of that) the default one but well, there are things we learn along the way.

What do you think?

PatTheMav commented 8 months ago

Probably best to document this to make it explicit that inline favouring will not apply to set (and I'd guess also list(APPEND ...) with more than 4 elements.

It's fine to have these arbitrary internal rules, though if documented they can cut down on issue reports like this one.