This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Fix treating non-space whitespaces in checks list.
ClosedPublic

Authored by curdeius on Mar 3 2017, 3:49 AM.

Details

Summary

This furtherly improves r295303: [clang-tidy] Ignore spaces between globs in the Checks option.
Trims all whitespaces and not only spaces and correctly computes the offset of the checks list (taking the size before trimming).

Event Timeline

curdeius created this revision.Mar 3 2017, 3:49 AM
alexfh edited edge metadata.Mar 3 2017, 5:28 AM

What's the practical use of newlines and tab characters in the glob list?

curdeius added a comment.EditedMar 9 2017, 2:55 AM

Hi Alex and sorry for the late reply.

The main use case is a more readable .clang-tidy configuration checks.
Before this correction one can use something like this:

---
Checks: '
    ,*,
    ,-cert-dcl03-c,
'
...

It works, but is hardly comprehensible to a newbie (the strange use of addtional commas).
Since the spaces are ignored (since a recent commit of yours) we can add spaces after the leading comma and hope that no user uses a tab...

After applying this patch, we can just write (with tabs or spaces and as many newlines as we want - used for grouping for instance):

---
Checks: '
    *,
    -cert-dcl03-c,
'
...

Additionaly, you can sometimes accidentally issue a tabulator on the command line and that's just nice to ignore it.

Hi Alex and sorry for the late reply.

The main use case is a more readable .clang-tidy configuration checks.
Before this correction one can use something like this:

---
Checks: '
    ,*,
    ,-cert-dcl03-c,
'
...

It works, but is hardly comprehensible to a newbie (the strange use of addtional commas).
Since the spaces are ignored (since a recent commit of yours) we can add spaces after the leading comma and hope that no user uses a tab...

After applying this patch, we can just write (with tabs or spaces and as many newlines as we want - used for grouping for instance):

---
Checks: '
    *,
    -cert-dcl03-c,
'
...

Additionaly, you can sometimes accidentally issue a tabulator on the command line and that's just nice to ignore it.

Currently, one can use YAML folded strings to split the list of checks to multiple lines:

Checks: >
  -*,
  cert-dcl03-c

So it's not necessary to trim newlines. And tabs are forbidden in YAML: http://www.yaml.org/faq.html. So I'd suggest to leave .trim(' '). The rest of the change looks good - thanks for catching the bug ;)

curdeius updated this revision to Diff 92630.Mar 22 2017, 6:45 AM

Trim only spaces.

Hi Alex, is it OK now?

There's one more trim() you missed. And the test needs to be updated (s/\\n/ /).

clang-tidy/ClangTidyDiagnosticConsumer.cpp
133

s/trim()/trim(' ')/

curdeius updated this revision to Diff 92772.Mar 23 2017, 2:28 AM

Trim spaces only everywhere. Fix test.

alexfh accepted this revision.Mar 23 2017, 8:25 AM

LG. Thanks!

Do you have commit rights?

This revision is now accepted and ready to land.Mar 23 2017, 8:25 AM

Do you have commit rights?

Yes, I will commit this ASAP.

curdeius closed this revision.Mar 23 2017, 9:44 AM