| Message ID | 20211117193320.77418-1-mkp@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v1] checkpatch: Correct line count in error messages | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
Mike Pattrick <mkp@redhat.com> writes: > As part of some previous checkpatch work, we discovered that checkpatch > isn't always reporting correct line numbers. As it turns out, Python's > splitlines function considers several characters to be new lines which > common text editors do not typically consider to be new lines. For > example, form feed characters, which this code base uses to cluster > functionality. > > To retain a very similar functionality to what exists now, we will > split on "\r?\n" and strip off the other characters that previously > splitlines was splitting on. > > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- Thanks for the fix. I noticed it when I ran $ ./utilities/checkpatch.py -f ofproto/ofproto.c and the line numbers started to drift. With this change, I don't see that behavior any longer. Acked-by: Aaron Conole <aconole@redhat.com>
On 11/19/21 15:15, Aaron Conole wrote: > Mike Pattrick <mkp@redhat.com> writes: > >> As part of some previous checkpatch work, we discovered that checkpatch >> isn't always reporting correct line numbers. As it turns out, Python's >> splitlines function considers several characters to be new lines which >> common text editors do not typically consider to be new lines. For >> example, form feed characters, which this code base uses to cluster >> functionality. >> >> To retain a very similar functionality to what exists now, we will >> split on "\r?\n" and strip off the other characters that previously >> splitlines was splitting on. Looks way too complex to me. Can we just skip and not count lines with a single ^L on them instead? They are supposed to be on their own lines. And there should be no other control characters in the code. Best regards, Ilya Maximets. >> >> Signed-off-by: Mike Pattrick <mkp@redhat.com> >> --- > > Thanks for the fix. I noticed it when I ran > > $ ./utilities/checkpatch.py -f ofproto/ofproto.c > > and the line numbers started to drift. With this change, I don't see > that behavior any longer. > > Acked-by: Aaron Conole <aconole@redhat.com> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index bf95358d5..2a3c0f768 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -765,12 +765,13 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): reset_counters() - for line in text.splitlines(): + for line in re.split("\r?\n", text): if current_file != previous_file: previous_file = current_file lineno = lineno + 1 total_line = total_line + 1 + line = line.rstrip("\u2029\u2028\x85\x1e\x1d\x1c\x0c\x0b") if len(line) <= 0: continue
As part of some previous checkpatch work, we discovered that checkpatch isn't always reporting correct line numbers. As it turns out, Python's splitlines function considers several characters to be new lines which common text editors do not typically consider to be new lines. For example, form feed characters, which this code base uses to cluster functionality. To retain a very similar functionality to what exists now, we will split on "\r?\n" and strip off the other characters that previously splitlines was splitting on. Signed-off-by: Mike Pattrick <mkp@redhat.com> --- utilities/checkpatch.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)