cmake_format icon indicating copy to clipboard operation
cmake_format copied to clipboard

Cmake-lint: ignore long links

Open okainov opened this issue 5 years ago • 4 comments

I'm getting cmake-lint issue like cmake/mycmake.cmake:08: [C0301] Line too long (97/80) while my cmake file has something like

ExternalProject_Add(
  mything
  GIT_REPOSITORY
    https://gitlab-mirror-ir.long.corporate.domain.com/longgroup/path/to/somewhere/myteam/mything.git
  GIT_SHALLOW ON

Please detect links and don't fail lint when there is link longer than line length limit...

okainov avatar Apr 29 '20 15:04 okainov

Hm... I'll need to think about this a bit. Honestly, I would consider this lint, it can be addressed by, e.g.:

set(_base_url https://gitlab-mirror-ir.long.corporate.domain.com/longgroup)

ExternalProject_Add(
  mything
  GIT_REPOSITORY
    ${_base_url}/path/to/somewhere/myteam/mything.git
  GIT_SHALLOW ON

cheshirekow avatar May 01 '20 22:05 cheshirekow

It's the same as long links for cmake-format (#195, #182), I'd like to keep URL as one thing so it's possible to click on it from modern text editors and\or copy-paste into browser as is. Also, the behavior between cmake-format and cmake-lint should be consistent

okainov avatar May 02 '20 08:05 okainov

It's the same as long links for cmake-format (#195, #182),

No, this is really different. Those other issues are about whether or not cmake-format should modify existing text content, and have to do with how the content it tokenized. cmake-lint allowing "special" tokens to violate style rules is a different thing altogether.

I'd like to keep URL as one thing...

In general, if it allowable within a project's style guide to violate a particular rule in a special case, then you can locally disable the check. What you are requesting though, "cmake-lint should support recognition of special cases allowed to break a rule", I have some concerns about:

  1. How many special cases should it understand?
  2. How many rules should be special-caseable?
  3. How should cmake-lint associate "special cases" with "rules allowed to violate"?

I'll have to think about it more. Are long URLs common enough to make a one-off feature? Is this going to grow into something more complicated? I'm not sure yet. The time is probably better spent on making #cmake-lint: disable= more granular and expressive.

If you want something clickable, you could always use a URL shortener:

# https://tinyurl.com/y7dyl65b
string(JOIN "/" _url 
  https://gitlab-mirror-ir.long.corporate.domain.com
  longgroup/path/to/somewhere
  myteam/mything.git)

ExternalProject_Add(
  mything
  GIT_REPOSITORY ${_url}
  GIT_SHALLOW ON
  ...

Also, the behavior between cmake-format and cmake-lint should be consistent

They are currently consistent. cmake-format considers this a "failure to layout" as it could not achieve the desired line-width, and leaves things in the state of the "most agressive formatting" that tried. If you run with --require-valid-layout=true then cmake-format will also exit with nonzero status code.

cheshirekow avatar May 05 '20 21:05 cheshirekow

Some notes (mostly to self):

Currently this check is implemented during the first phase of checks before even lexing the input file. In order to implement this feature, this check will have to move after the parse. We would need two kinds of checks for (1) comments and (2) semantic tokens.

Plan: for either case, and if the token range extends beyond the column limit, then check the following: If the token spelling matches a user-defined regex pattern and it is the only non-whitespace token on it's line, then do not classify it as lint. Otherwise, classify it as lint.

We'll need to maintain some state to know if we have already found this lint on this line so we do not emit lint for each word after the first one which violates the column limit.

cheshirekow avatar Aug 13 '20 16:08 cheshirekow