cmake style

[dlyr]

Currently there is a cmake-lint configuration describe here https://github.com/STORM-IRIT/Radium-Engine/blob/master/.cmake-format It defines naming convention, which are not followed. Here are the rules

  function_pattern = '[0-9a-z_]+'
  macro_pattern = '[0-9a-z_]+'
  global_var_pattern = '[A-Z][0-9A-Z_]+'
  internal_var_pattern = '_[A-Z][0-9A-Z_]+'
  local_var_pattern = '[a-z][a-z0-9_]+'
  private_var_pattern = '_[0-9a-z_]+'
  public_var_pattern = '[A-Z][0-9A-Z_]+'
  argument_var_pattern = '[a-z][a-z0-9_]+'
  keyword_pattern = '[A-Z][0-9A-Z_]+'

It barely corresponds to cmake-format default, except for macro pattern, that is set to match default function pattern.

PR #958 propose to add cmake-lint as pre-commit, but it can be removed from the PR and added later, with style fix. Before that, we can discuss/validate these naming convention. (to skip one pre-commit check simpy use SKIP=name-of-the-check git commit -m"message" see https://pre-commit.com/#temporarily-disabling-hooks)

[dlyr]

I have updated public_var_pattern = '[A-Z][0-9A-Z_]+|([A-Za-z][0-9A-Za-z_]+)?_(NOTFOUND|FOUND|DIR)[0-9A-Za-z_]*' so we can set variables like Radium_FOUND

[dlyr]

I have removed the need for comment, after all for most custom target, I think we don't need comments. I have one concern with line length, which is not well handled for fenced comments, and ask to break urls. Also we can disable this check locally, but it remain active for the end of the scope (cmake-lint limitation).

Last concern is about public/private variable. I just find that there is a use of some variable as control to called function, even in a function, which I think is a bit odd (e.g. IsMacBundle in release-candidate). Should we avoid these ?

The proposed convention is to have lower_case_variable for set defined in local (function) scope and UPPER_CASE_NAME for variable set at global scope.

[dlyr]

Also line break force to have some wierd line break in custom command code. Maybe we can just ignore long lines and let coders do their best not to have too long lines.

[MathiasPaulin]

I just find that there is a use of some variable as control to called function, even in a function, which I think is a bit odd (e.g. IsMacBundle in release-candidate).

I'm not sure to understand what you mean ...

[dlyr]

on cmake-lint side, variables are of two kind, global (directory) or local. set used in function are local to this function, but are also defined as "global" if another function, in a subcall, use this variable.

If I stick to the current cmake-lint style, we may have

set(GLOBAL_VAR "hey")
function(foo)
  message("${local_variable}") # Set in another local scope
  message("${GLOBAL_VAR}") # Set globally
endfunction

function(bar)
  set(local_variable "ho")
  foo()
endfunction()

I think that local_variable should have been passed as argument, instead of be read from current scope.

[MathiasPaulin]

I do not understand why we need to make this difference nor why it is a problem. Variables are visible in the scope they are declared into and all the enclosed scope.

I've concerns about restricting the variables to be either purely local to a function or passed as parameters. All variables visible in a scope should be usable in this scope regardless where they come from.

That said, I'm no really keen by this kind of questions, so, the only thing I'll be attentive about is the usage simplicity for the developer.

[dlyr]

We need to make a difference because the linter do. Either we set all variable names UP_CASE, or down_case or CamelCase and tell the linter to enforce that, either we have a naming rule the linter can't enforce.

Or we can simply close the issue/PR and let every one write without guidelines nor enforcement, and have the naming inconsistency currently present in our cmake configuration (e.g. MIXED_upAs_itStheCaseNow)

[dlyr]

After some reflection, let try to do our best without linter, since current cmake-linter is not flexible enough.