Skip to content

Remove curly brace CPP Lint checks #2109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 25, 2018
Merged

Remove curly brace CPP Lint checks #2109

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d344dd9
Update coding standard
Apr 24, 2018
692c4f3
Remove brace checking from cpplint
Apr 24, 2018
78191ee
Remove NOLINTs for lambdas.
Apr 24, 2018
18cab61
Rephrase and justify curly brace alignment exceptions
Apr 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions scripts/cpplint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3972,14 +3972,6 @@ def CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error):
error(filename, linenum, 'whitespace/braces', 5,
'Missing space before {')

# Make sure '} else {' has spaces.
# if Search(r'}else', line):
# error(filename, linenum, 'whitespace/braces', 5,
# 'Missing space before else')
if (Search(r'^.*[^\s].*}$', line) or Search(r'^.*[^\s].*{$', line)) and not(Search(r'{[^}]*}', line)):
error(filename, linenum, 'whitespace/braces', 5,
'Put braces on a separate next line')

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this also remove the check for all compound statements such as loops, non-lambda function bodies, if/else? How about, instead of completely removing it, adding a not(Search(r'\]\s*\(.*\)\s*{', line))?

Copy link
Contributor Author

@LAJW LAJW Apr 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work in the case of lambda with a return type:

[] (T& t) -> optionalt<std::string> { ...

Or with a lambda with omitted arguments (used in named scopes):

auto value = [&] {
  if (this)
    return 1;
  if (that)
    return 2;
  return 3;
}();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't be very hard to add expressions for those, but I do worry about the various line breaks that could be used.

# You shouldn't have a space before a semicolon at the end of the line.
# There's a special case for "for" since the style guide allows space before
# the semicolon there.
Expand Down