pcl icon indicating copy to clipboard operation
pcl copied to clipboard

[ClangFormat] Add CaseSensitive to rule for Qt

Open SunBlack opened this issue 4 years ago • 2 comments

Currently the .clang-format file contains

IncludeCategories:
...
# Major 3rd-Party components of apps
  - Regex:           '^<Q[^/]+>$'
    Priority:        300
...
# Matches all std includes. Match them before any unknown include, so we can order them behind.
  - Regex:           '^<[a-z_]+>$'
    Priority:        900

Sadly the std include <queue> will be matched by the first rule, as the regular expressions are not case sensitive. This issue can be simply resolved by adding CaseSensitive: true to the rule for Qt - but this requires at least ClangFormat 12 (as far as I see the CI is currently using ClangFormat 10 of Ubuntu 20.04). Another option would be to adjust the regular expression, but as 22.04 will be released in a few weeks in may be easier to upgrade to 22.04 and use ClangFormat 14.0 than.

SunBlack avatar Mar 25 '22 16:03 SunBlack

Either way seems good to me, feel free to create a pull request with whichever solution you think is best. Are you aware of a file in PCL where this has already led to a wrong ordering?

mvieth avatar Mar 25 '22 17:03 mvieth

Tested it (with ClangFormat 13): The PCL does have only 8x #include <queue> within its source, but nowhere Qt is also a dependencies. So this is only a nice-to-have fix to prevent issues maybe in the future, but currently not relevant.

In general it would be more important to update ClangFormat due to incompatibilities between the versions (sadly they don't provide any backward compatibility). Somehow ClangFormat-9 is part of 18.04-22.04, but ClangFormat-10 only of 18.04 & 20.04. So I can't use e.g. my 21.10-VM to check the formatting :-/.

So this is just something we should remember to fix when upgrading ClangFormat, but otherwise not relevant.

SunBlack avatar Mar 28 '22 10:03 SunBlack