Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On 2018-07-16 11:20, pher...@codeaurora.org wrote:
On 2018-07-13 17:08, Joe Perches wrote:
On Fri, 2018-07-13 at 16:28 -0700, pher...@codeaurora.org wrote:
On 2018-07-13 14:46, Joe Perches wrote:
> On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote:
> > Commit text is almost always necessary to explain why a change is
> > needed.
>
> This bit seems sensible, but perhaps it should just count the
> number of lines after the end of email headers and before any
> Signed-off-by:/Signature line
>
While committing the changes, one can just write the subject and not
write
the commit text at all. So, if we just count the lines between email
headers
and signed-off, we still do count lines which form the subject, but
the
commit text is still absent. Also, subject can be longer than one
line.
So,
just counting lines doesn't really guarantee the presence of commit
text.
Not true.
Look at $in_header_lines and $in_commit_log.
> > Also, warn on commit text lines longer than 75 characters. The commit
> > text
> > are indented and may wrap on a terminal if they are longer than 75
> > characters.
>
> This is already exists via
>
> # Check for line lengths > 75 in commit log, warn once
>if ($in_commit_log && !$commit_log_long_line &&
>length($line) > 75 &&
>
True, but this patch points out every line of the commit text that is
exceeding the limit.
Which is bad because things like dump_stack() are added in
commit logs and those are already allowed to be > 75 chars.
Anyway, something like this probably works. Please test.
---
scripts/checkpatch.pl | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b5c875d7132b..8b5f3dae31c9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
+ my $commit_log_lines = 0; #Number of commit log lines
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2497,6 +2498,18 @@ sub process {
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate
+# 2 is used because a $signature is counted in $commit_log_lines
+ if ($in_commit_log) {
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
+ } else if ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+"Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
+ }
+
# Check if the commit log has what seems like a diff which can
confuse patch
if ($in_commit_log && !$commit_log_has_diff &&
(($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
I checked all the cases that I mentioned before. The change you
suggested works
for every case. Would you take care of merging this fix?
Any updates on this patch? Would you take care of merging this fix?
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On 2018-07-16 11:20, pher...@codeaurora.org wrote:
On 2018-07-13 17:08, Joe Perches wrote:
On Fri, 2018-07-13 at 16:28 -0700, pher...@codeaurora.org wrote:
On 2018-07-13 14:46, Joe Perches wrote:
> On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote:
> > Commit text is almost always necessary to explain why a change is
> > needed.
>
> This bit seems sensible, but perhaps it should just count the
> number of lines after the end of email headers and before any
> Signed-off-by:/Signature line
>
While committing the changes, one can just write the subject and not
write
the commit text at all. So, if we just count the lines between email
headers
and signed-off, we still do count lines which form the subject, but
the
commit text is still absent. Also, subject can be longer than one
line.
So,
just counting lines doesn't really guarantee the presence of commit
text.
Not true.
Look at $in_header_lines and $in_commit_log.
> > Also, warn on commit text lines longer than 75 characters. The commit
> > text
> > are indented and may wrap on a terminal if they are longer than 75
> > characters.
>
> This is already exists via
>
> # Check for line lengths > 75 in commit log, warn once
>if ($in_commit_log && !$commit_log_long_line &&
>length($line) > 75 &&
>
True, but this patch points out every line of the commit text that is
exceeding the limit.
Which is bad because things like dump_stack() are added in
commit logs and those are already allowed to be > 75 chars.
Anyway, something like this probably works. Please test.
---
scripts/checkpatch.pl | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b5c875d7132b..8b5f3dae31c9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
+ my $commit_log_lines = 0; #Number of commit log lines
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2497,6 +2498,18 @@ sub process {
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate
+# 2 is used because a $signature is counted in $commit_log_lines
+ if ($in_commit_log) {
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
+ } else if ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+"Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
+ }
+
# Check if the commit log has what seems like a diff which can
confuse patch
if ($in_commit_log && !$commit_log_has_diff &&
(($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
I checked all the cases that I mentioned before. The change you
suggested works
for every case. Would you take care of merging this fix?
Any updates on this patch? Would you take care of merging this fix?
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On 2018-07-13 17:08, Joe Perches wrote:
On Fri, 2018-07-13 at 16:28 -0700, pher...@codeaurora.org wrote:
On 2018-07-13 14:46, Joe Perches wrote:
> On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote:
> > Commit text is almost always necessary to explain why a change is
> > needed.
>
> This bit seems sensible, but perhaps it should just count the
> number of lines after the end of email headers and before any
> Signed-off-by:/Signature line
>
While committing the changes, one can just write the subject and not
write
the commit text at all. So, if we just count the lines between email
headers
and signed-off, we still do count lines which form the subject, but
the
commit text is still absent. Also, subject can be longer than one
line.
So,
just counting lines doesn't really guarantee the presence of commit
text.
Not true.
Look at $in_header_lines and $in_commit_log.
> > Also, warn on commit text lines longer than 75 characters. The commit
> > text
> > are indented and may wrap on a terminal if they are longer than 75
> > characters.
>
> This is already exists via
>
> # Check for line lengths > 75 in commit log, warn once
>if ($in_commit_log && !$commit_log_long_line &&
>length($line) > 75 &&
>
True, but this patch points out every line of the commit text that is
exceeding the limit.
Which is bad because things like dump_stack() are added in
commit logs and those are already allowed to be > 75 chars.
Anyway, something like this probably works. Please test.
---
scripts/checkpatch.pl | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b5c875d7132b..8b5f3dae31c9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
+ my $commit_log_lines = 0; #Number of commit log lines
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2497,6 +2498,18 @@ sub process {
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate
+# 2 is used because a $signature is counted in $commit_log_lines
+ if ($in_commit_log) {
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
+ } else if ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+"Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
+ }
+
# Check if the commit log has what seems like a diff which can confuse
patch
if ($in_commit_log && !$commit_log_has_diff &&
(($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
I checked all the cases that I mentioned before. The change you
suggested works
for every case. Would you take care of merging this fix?
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On 2018-07-13 17:08, Joe Perches wrote:
On Fri, 2018-07-13 at 16:28 -0700, pher...@codeaurora.org wrote:
On 2018-07-13 14:46, Joe Perches wrote:
> On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote:
> > Commit text is almost always necessary to explain why a change is
> > needed.
>
> This bit seems sensible, but perhaps it should just count the
> number of lines after the end of email headers and before any
> Signed-off-by:/Signature line
>
While committing the changes, one can just write the subject and not
write
the commit text at all. So, if we just count the lines between email
headers
and signed-off, we still do count lines which form the subject, but
the
commit text is still absent. Also, subject can be longer than one
line.
So,
just counting lines doesn't really guarantee the presence of commit
text.
Not true.
Look at $in_header_lines and $in_commit_log.
> > Also, warn on commit text lines longer than 75 characters. The commit
> > text
> > are indented and may wrap on a terminal if they are longer than 75
> > characters.
>
> This is already exists via
>
> # Check for line lengths > 75 in commit log, warn once
>if ($in_commit_log && !$commit_log_long_line &&
>length($line) > 75 &&
>
True, but this patch points out every line of the commit text that is
exceeding the limit.
Which is bad because things like dump_stack() are added in
commit logs and those are already allowed to be > 75 chars.
Anyway, something like this probably works. Please test.
---
scripts/checkpatch.pl | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b5c875d7132b..8b5f3dae31c9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
+ my $commit_log_lines = 0; #Number of commit log lines
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2497,6 +2498,18 @@ sub process {
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate
+# 2 is used because a $signature is counted in $commit_log_lines
+ if ($in_commit_log) {
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
+ } else if ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+"Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
+ }
+
# Check if the commit log has what seems like a diff which can confuse
patch
if ($in_commit_log && !$commit_log_has_diff &&
(($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
I checked all the cases that I mentioned before. The change you
suggested works
for every case. Would you take care of merging this fix?
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On Fri, 2018-07-13 at 16:28 -0700, pher...@codeaurora.org wrote:
> On 2018-07-13 14:46, Joe Perches wrote:
> > On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote:
> > > Commit text is almost always necessary to explain why a change is
> > > needed.
> >
> > This bit seems sensible, but perhaps it should just count the
> > number of lines after the end of email headers and before any
> > Signed-off-by:/Signature line
> >
>
> While committing the changes, one can just write the subject and not
> write
> the commit text at all. So, if we just count the lines between email
> headers
> and signed-off, we still do count lines which form the subject, but the
> commit text is still absent. Also, subject can be longer than one line.
> So,
> just counting lines doesn't really guarantee the presence of commit
> text.
Not true.
Look at $in_header_lines and $in_commit_log.
> > > Also, warn on commit text lines longer than 75 characters. The commit
> > > text
> > > are indented and may wrap on a terminal if they are longer than 75
> > > characters.
> >
> > This is already exists via
> >
> > # Check for line lengths > 75 in commit log, warn once
> > if ($in_commit_log && !$commit_log_long_line &&
> > length($line) > 75 &&
> >
>
> True, but this patch points out every line of the commit text that is
> exceeding the limit.
Which is bad because things like dump_stack() are added in
commit logs and those are already allowed to be > 75 chars.
Anyway, something like this probably works. Please test.
---
scripts/checkpatch.pl | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b5c875d7132b..8b5f3dae31c9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
+ my $commit_log_lines = 0; #Number of commit log lines
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2497,6 +2498,18 @@ sub process {
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate
+# 2 is used because a $signature is counted in $commit_log_lines
+ if ($in_commit_log) {
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
+ } else if ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+"Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
+ }
+
# Check if the commit log has what seems like a diff which can confuse patch
if ($in_commit_log && !$commit_log_has_diff &&
(($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On Fri, 2018-07-13 at 16:28 -0700, pher...@codeaurora.org wrote:
> On 2018-07-13 14:46, Joe Perches wrote:
> > On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote:
> > > Commit text is almost always necessary to explain why a change is
> > > needed.
> >
> > This bit seems sensible, but perhaps it should just count the
> > number of lines after the end of email headers and before any
> > Signed-off-by:/Signature line
> >
>
> While committing the changes, one can just write the subject and not
> write
> the commit text at all. So, if we just count the lines between email
> headers
> and signed-off, we still do count lines which form the subject, but the
> commit text is still absent. Also, subject can be longer than one line.
> So,
> just counting lines doesn't really guarantee the presence of commit
> text.
Not true.
Look at $in_header_lines and $in_commit_log.
> > > Also, warn on commit text lines longer than 75 characters. The commit
> > > text
> > > are indented and may wrap on a terminal if they are longer than 75
> > > characters.
> >
> > This is already exists via
> >
> > # Check for line lengths > 75 in commit log, warn once
> > if ($in_commit_log && !$commit_log_long_line &&
> > length($line) > 75 &&
> >
>
> True, but this patch points out every line of the commit text that is
> exceeding the limit.
Which is bad because things like dump_stack() are added in
commit logs and those are already allowed to be > 75 chars.
Anyway, something like this probably works. Please test.
---
scripts/checkpatch.pl | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b5c875d7132b..8b5f3dae31c9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $has_commit_log = 0; #Encountered lines before patch
+ my $commit_log_lines = 0; #Number of commit log lines
my $commit_log_possible_stack_dump = 0;
my $commit_log_long_line = 0;
my $commit_log_has_diff = 0;
@@ -2497,6 +2498,18 @@ sub process {
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate
+# 2 is used because a $signature is counted in $commit_log_lines
+ if ($in_commit_log) {
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
+ } else if ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+"Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
+ }
+
# Check if the commit log has what seems like a diff which can confuse patch
if ($in_commit_log && !$commit_log_has_diff &&
(($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On 2018-07-13 14:46, Joe Perches wrote: On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote: Commit text is almost always necessary to explain why a change is needed. This bit seems sensible, but perhaps it should just count the number of lines after the end of email headers and before any Signed-off-by:/Signature line While committing the changes, one can just write the subject and not write the commit text at all. So, if we just count the lines between email headers and signed-off, we still do count lines which form the subject, but the commit text is still absent. Also, subject can be longer than one line. So, just counting lines doesn't really guarantee the presence of commit text. Also, warn on commit text lines longer than 75 characters. The commit text are indented and may wrap on a terminal if they are longer than 75 characters. This is already exists via # Check for line lengths > 75 in commit log, warn once if ($in_commit_log && !$commit_log_long_line && length($line) > 75 && True, but this patch points out every line of the commit text that is exceeding the limit. diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] @@ -14,6 +14,13 @@ use File::Basename; use Cwd 'abs_path'; use Term::ANSIColor qw(:constants); +use constant BEFORE_SHORTTEXT => 0; +use constant IN_SHORTTEXT_BLANKLINE => 1; +use constant IN_SHORTTEXT => 2; +use constant AFTER_SHORTTEXT => 3; +use constant CHECK_NEXT_SHORTTEXT => 4; +use constant SHORTTEXT_LIMIT => 75; probably overly complicated
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On 2018-07-13 14:46, Joe Perches wrote: On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote: Commit text is almost always necessary to explain why a change is needed. This bit seems sensible, but perhaps it should just count the number of lines after the end of email headers and before any Signed-off-by:/Signature line While committing the changes, one can just write the subject and not write the commit text at all. So, if we just count the lines between email headers and signed-off, we still do count lines which form the subject, but the commit text is still absent. Also, subject can be longer than one line. So, just counting lines doesn't really guarantee the presence of commit text. Also, warn on commit text lines longer than 75 characters. The commit text are indented and may wrap on a terminal if they are longer than 75 characters. This is already exists via # Check for line lengths > 75 in commit log, warn once if ($in_commit_log && !$commit_log_long_line && length($line) > 75 && True, but this patch points out every line of the commit text that is exceeding the limit. diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] @@ -14,6 +14,13 @@ use File::Basename; use Cwd 'abs_path'; use Term::ANSIColor qw(:constants); +use constant BEFORE_SHORTTEXT => 0; +use constant IN_SHORTTEXT_BLANKLINE => 1; +use constant IN_SHORTTEXT => 2; +use constant AFTER_SHORTTEXT => 3; +use constant CHECK_NEXT_SHORTTEXT => 4; +use constant SHORTTEXT_LIMIT => 75; probably overly complicated
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote: > Commit text is almost always necessary to explain why a change is needed. This bit seems sensible, but perhaps it should just count the number of lines after the end of email headers and before any Signed-off-by:/Signature line > Also, warn on commit text lines longer than 75 characters. The commit text > are indented and may wrap on a terminal if they are longer than 75 > characters. This is already exists via # Check for line lengths > 75 in commit log, warn once if ($in_commit_log && !$commit_log_long_line && length($line) > 75 && > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -14,6 +14,13 @@ use File::Basename; > use Cwd 'abs_path'; > use Term::ANSIColor qw(:constants); > > +use constant BEFORE_SHORTTEXT => 0; > +use constant IN_SHORTTEXT_BLANKLINE => 1; > +use constant IN_SHORTTEXT => 2; > +use constant AFTER_SHORTTEXT => 3; > +use constant CHECK_NEXT_SHORTTEXT => 4; > +use constant SHORTTEXT_LIMIT => 75; probably overly complicated
Re: [PATCH] checkpatch: Require commit text and warn on long commit text lines
On Fri, 2018-07-13 at 14:40 -0700, Prakruthi Deepak Heragu wrote: > Commit text is almost always necessary to explain why a change is needed. This bit seems sensible, but perhaps it should just count the number of lines after the end of email headers and before any Signed-off-by:/Signature line > Also, warn on commit text lines longer than 75 characters. The commit text > are indented and may wrap on a terminal if they are longer than 75 > characters. This is already exists via # Check for line lengths > 75 in commit log, warn once if ($in_commit_log && !$commit_log_long_line && length($line) > 75 && > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -14,6 +14,13 @@ use File::Basename; > use Cwd 'abs_path'; > use Term::ANSIColor qw(:constants); > > +use constant BEFORE_SHORTTEXT => 0; > +use constant IN_SHORTTEXT_BLANKLINE => 1; > +use constant IN_SHORTTEXT => 2; > +use constant AFTER_SHORTTEXT => 3; > +use constant CHECK_NEXT_SHORTTEXT => 4; > +use constant SHORTTEXT_LIMIT => 75; probably overly complicated
[PATCH] checkpatch: Require commit text and warn on long commit text lines
Commit text is almost always necessary to explain why a change is needed.
Also, warn on commit text lines longer than 75 characters. The commit text
are indented and may wrap on a terminal if they are longer than 75
characters.
Signed-off-by: David Keitel
Signed-off-by: Prakruthi Deepak Heragu
---
scripts/checkpatch.pl | 92 ++-
1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a9c0550..336a8e5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -14,6 +14,13 @@ use File::Basename;
use Cwd 'abs_path';
use Term::ANSIColor qw(:constants);
+use constant BEFORE_SHORTTEXT => 0;
+use constant IN_SHORTTEXT_BLANKLINE => 1;
+use constant IN_SHORTTEXT => 2;
+use constant AFTER_SHORTTEXT => 3;
+use constant CHECK_NEXT_SHORTTEXT => 4;
+use constant SHORTTEXT_LIMIT => 75;
+
my $P = $0;
my $D = dirname(abs_path($P));
@@ -2227,6 +2234,8 @@ sub process {
my $prevrawline="";
my $stashline="";
my $stashrawline="";
+ my $subjectline="";
+ my $sublinenr="";
my $length;
my $indent;
@@ -2282,6 +2291,9 @@ sub process {
my $setup_docs = 0;
my $camelcase_file_seeded = 0;
+ my $shorttext = BEFORE_SHORTTEXT;
+ my $shorttext_exspc = 0;
+ my $commit_text_present = 0;
my $checklicenseline = 1;
@@ -2487,13 +2499,91 @@ sub process {
$checklicenseline = 1;
next;
}
-
$here .= "FILE: $realfile:$realline:" if ($realcnt != 0);
my $hereline = "$here\n$rawline\n";
my $herecurr = "$here\n$rawline\n";
my $hereprev = "$here\n$prevrawline\n$rawline\n";
+ if ($shorttext != AFTER_SHORTTEXT) {
+ if ($shorttext == IN_SHORTTEXT_BLANKLINE &&
$line=~/\S/) {
+ # the subject line was just processed,
+ # a blank line must be next
+ $shorttext = IN_SHORTTEXT;
+ # this non-blank line may or may not be commit
text -
+ # a warning has been generated so assume it is
commit
+ # text and move on
+ $commit_text_present = 1;
+ # fall through and treat this line as
IN_SHORTTEXT
+ }
+ if ($shorttext == IN_SHORTTEXT) {
+ if ($line=~/^---/ || $line=~/^diff.*/) {
+ if ($commit_text_present == 0) {
+ WARN("NO_COMMIT_TEXT",
+"please add commit text
explaining " .
+"*why* the change is
needed\n" .
+$herecurr);
+ }
+ $shorttext = AFTER_SHORTTEXT;
+ } elsif (length($line) > (SHORTTEXT_LIMIT +
+ $shorttext_exspc)
+&& $line !~ /^:([0-7]{6}\s){2}
+ ([[:xdigit:]]+\.*
+ \s){2}\w+\s\w+/xms) {
+ WARN("LONG_COMMIT_TEXT",
+"commit text line over " .
+SHORTTEXT_LIMIT .
+" characters\n" . $herecurr);
+ $commit_text_present = 1;
+ } elsif ($line=~/^\s*[\x21-\x39\x3b-\x7e]+:/) {
+ # this is a tag, there must be commit
+ # text by now
+ if ($commit_text_present == 0) {
+ WARN("NO_COMMIT_TEXT",
+"please add commit text
explaining " .
+"*why* the change is
needed\n" .
+$herecurr);
+ # prevent duplicate warnings
+ $commit_text_present = 1;
+ }
+ } elsif ($line=~/\S/) {
+ $commit_text_present = 1;
+ }
+ } elsif ($shorttext == IN_SHORTTEXT_BLANKLINE) {
+ # case of non-blank line in this state handled
above
+
[PATCH] checkpatch: Require commit text and warn on long commit text lines
Commit text is almost always necessary to explain why a change is needed.
Also, warn on commit text lines longer than 75 characters. The commit text
are indented and may wrap on a terminal if they are longer than 75
characters.
Signed-off-by: David Keitel
Signed-off-by: Prakruthi Deepak Heragu
---
scripts/checkpatch.pl | 92 ++-
1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a9c0550..336a8e5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -14,6 +14,13 @@ use File::Basename;
use Cwd 'abs_path';
use Term::ANSIColor qw(:constants);
+use constant BEFORE_SHORTTEXT => 0;
+use constant IN_SHORTTEXT_BLANKLINE => 1;
+use constant IN_SHORTTEXT => 2;
+use constant AFTER_SHORTTEXT => 3;
+use constant CHECK_NEXT_SHORTTEXT => 4;
+use constant SHORTTEXT_LIMIT => 75;
+
my $P = $0;
my $D = dirname(abs_path($P));
@@ -2227,6 +2234,8 @@ sub process {
my $prevrawline="";
my $stashline="";
my $stashrawline="";
+ my $subjectline="";
+ my $sublinenr="";
my $length;
my $indent;
@@ -2282,6 +2291,9 @@ sub process {
my $setup_docs = 0;
my $camelcase_file_seeded = 0;
+ my $shorttext = BEFORE_SHORTTEXT;
+ my $shorttext_exspc = 0;
+ my $commit_text_present = 0;
my $checklicenseline = 1;
@@ -2487,13 +2499,91 @@ sub process {
$checklicenseline = 1;
next;
}
-
$here .= "FILE: $realfile:$realline:" if ($realcnt != 0);
my $hereline = "$here\n$rawline\n";
my $herecurr = "$here\n$rawline\n";
my $hereprev = "$here\n$prevrawline\n$rawline\n";
+ if ($shorttext != AFTER_SHORTTEXT) {
+ if ($shorttext == IN_SHORTTEXT_BLANKLINE &&
$line=~/\S/) {
+ # the subject line was just processed,
+ # a blank line must be next
+ $shorttext = IN_SHORTTEXT;
+ # this non-blank line may or may not be commit
text -
+ # a warning has been generated so assume it is
commit
+ # text and move on
+ $commit_text_present = 1;
+ # fall through and treat this line as
IN_SHORTTEXT
+ }
+ if ($shorttext == IN_SHORTTEXT) {
+ if ($line=~/^---/ || $line=~/^diff.*/) {
+ if ($commit_text_present == 0) {
+ WARN("NO_COMMIT_TEXT",
+"please add commit text
explaining " .
+"*why* the change is
needed\n" .
+$herecurr);
+ }
+ $shorttext = AFTER_SHORTTEXT;
+ } elsif (length($line) > (SHORTTEXT_LIMIT +
+ $shorttext_exspc)
+&& $line !~ /^:([0-7]{6}\s){2}
+ ([[:xdigit:]]+\.*
+ \s){2}\w+\s\w+/xms) {
+ WARN("LONG_COMMIT_TEXT",
+"commit text line over " .
+SHORTTEXT_LIMIT .
+" characters\n" . $herecurr);
+ $commit_text_present = 1;
+ } elsif ($line=~/^\s*[\x21-\x39\x3b-\x7e]+:/) {
+ # this is a tag, there must be commit
+ # text by now
+ if ($commit_text_present == 0) {
+ WARN("NO_COMMIT_TEXT",
+"please add commit text
explaining " .
+"*why* the change is
needed\n" .
+$herecurr);
+ # prevent duplicate warnings
+ $commit_text_present = 1;
+ }
+ } elsif ($line=~/\S/) {
+ $commit_text_present = 1;
+ }
+ } elsif ($shorttext == IN_SHORTTEXT_BLANKLINE) {
+ # case of non-blank line in this state handled
above
+
