search

cheshirekow / cmake_format

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

Allow configurable lint check to flag/change commands to uppercase/lowercase or as it currently is mixed-case #239

Open compor opened 3 years ago

compor commented 3 years ago

Hello,

First, thanks a lot for the work in this project.

TL;DR

Allow configurable lint check to flag/change commands to uppercase/lowercase or as it currently is mixed-case

Description

I came across this issue because I recently switch from cmake-lint to, well, cmakelint (this project v0.6.13). I'm using Vim with ALE for linting and will transitioning and trying out this linter I've hooked up both of them in order to get their recommendations. Consider this CMakeLists.txt:

cmake_minimum_required(VERSION 3.19)
project(issuereport C CXX)

include(FetchContent)

FetchContent_Declare(
    Catch2
    GIT_REPOSITORY https://github.com/catchorg/Catch2.git
    GIT_TAG v2.13.3)

FetchContent_MakeAvailable(Catch2)

list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/contrib)
include(Catch)

add_executable(unittests foo.cpp)                                                                                                                                                                                               

target_link_libraries(unittests Catch2::Catch2)

# make tests discoverable via CTest
catch_discover_tests(unittests)

I'd like the FetchContent_MakeAvailable to stay as or be transformed to fetchcontent_makeavailable. However, cmake-format converts it to the former and I haven't been able to find a relevant lint rule or option in the config to change this.

On the other hand, cmake-lint flags this via its readability/wonkycase rule.

Command case-insensitivity is one of the "features" of CMake that I hate and when mixing these it sort of makes it worse for me. I'm not familiar with a convention, since I've seen projects use all capitals (especially older ones), a mix or all lowercase. I'm of the latter school, since I really do think it improves readibility and avoids surprises since variables are case-sensitive, and I was wondering if it would be considered if an option for this was made preserving the current default behaviour for consistency with older releases of cmake_format & co., but allowing users like me to set this to their preference.

I'd be glad to discuss this further or point me to the right direction if it'd be preferrable to help with this otherwise.

cheshirekow commented 3 years ago

I think the option you are looking for is command_case. The option only applies to cmake-format, there is no corresponding lint rule (yet... as you have noticed in the lint db). I believe that does address your request though (e.g. cmake-format will make things consistent). The default is canonical. For the case of FetchContent_MakeAvailable the wonky case is canonical. I believe what you want is to change this option to lower.

Canonical spellings for each command are specified by the "spelling" field for a standard entry. There are also a couple that are implemented as overrides here.

compor commented 3 years ago

Yes, I think it does address my issue partly for now. I somehow hastily and erroneously assumed that the lint and format rule correspondence is there in all the tools.

Let me play a bit around with this and get back to you in order to close this.

How much interest would there be for the corresponding lint rule? Maybe I can muster a PR if I find the time. Any pointers welcome.

Thanks for getting back fast.

cheshirekow commented 3 years ago

Adding a command case lint check is reasonable. I think implementation can be fairly straightforward. Let me have a look at adding this in the next batch of changes.