Skip to content

Set a max of 1 consecutive blank line #7

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

Draft
EduPonz wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Set a max of 1 consecutive blank line #7

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion uncrustify.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1981,7 +1981,7 @@ nl_split_while_one_liner = false # true/false
#

# The maximum number of consecutive newlines (3 = 2 blank lines).
nl_max = 0 # unsigned number
nl_max = 2 # unsigned number

# The maximum number of consecutive newlines in a function.
nl_max_blank_in_func = 0 # unsigned number
Comment on lines +1984 to 1987
Copy link
Member

@MiguelCompany MiguelCompany Feb 22, 2023

Choose a reason for hiding this comment

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

Suggested change
nl_max = 2 # unsigned number
# The maximum number of consecutive newlines in a function.
nl_max_blank_in_func = 0 # unsigned number
nl_max = 3 # unsigned number
# The maximum number of consecutive newlines in a function.
nl_max_blank_in_func = 2 # unsigned number

Copy link
Author

@EduPonz EduPonz Feb 22, 2023

Choose a reason for hiding this comment

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

This would leave room to have either 1 or 2 blanks before things like function or class definitions, which IMO should be fixed to either 1 (I just used 1 because our code base does more or less that) or 2 (as for instance pep8 enforces for python). What I would not do is to leave it up to the contributor, I'd rather have some uniformity

Copy link

@jsan-rt jsan-rt Feb 22, 2023

Choose a reason for hiding this comment

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

Fair concern. We could use this set of configurations for that then:

# The number of newlines before a function prototype.
nl_before_func_body_proto       = 0        # unsigned number

# The number of newlines before a multi-line function definition.
nl_before_func_body_def         = 0        # unsigned number

# The number of newlines before a class constructor/destructor prototype.
nl_before_func_class_proto      = 0        # unsigned number

# The number of newlines before a class constructor/destructor definition.
nl_before_func_class_def        = 0        # unsigned number

Copy link
Member

@MiguelCompany MiguelCompany Feb 22, 2023

Choose a reason for hiding this comment

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

This was more a suggestion than a request (that's why I only commented).

I agree with your opinion, but we should take into account our whole code base, since this is the file for all the code in the eProsima repositories.

I haven't checked all of them, but I think you are right that we are almost always using a single blank line everywhere.

Copy link
Author

@EduPonz EduPonz Feb 22, 2023

Choose a reason for hiding this comment

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

Yes, definitely! That's why I'd like as many opinions as possible here

Expand Down