On 12/10/16 17:49, Martin Sebor wrote:
On 10/12/2016 06:43 AM, Kyrill Tkachov wrote:
On 12/10/16 11:18, Kyrill Tkachov wrote:
On 12/10/16 10:57, Kyrill Tkachov wrote:
On 11/10/16 20:19, Jakub Jelinek wrote:
On Tue, Oct 11, 2016 at 01:11:04PM -0600, Martin Sebor wrote:
Also, the pattern that starts with "/\+\+\+" looks like it's missing
the ^ anchor. Presumably it should be "/^\+\+\+ \/testsuite\//".
No, it will be almost never +++ /testsuite/
There needs to be .* in between "+++ " and "/testsuite/", and perhaps
it should also ignore "+++ testsuite/".
So /^\+\+\+ (.*\/)?testsuite\// ?
Also, normally (when matching $0) there won't be newlines in the text.
Jakub
Thanks.
Here is the updated patch with your suggestions.
Actually, I've encountered a problem:
85 # Remove the testsuite part of the diff. We don't care about GNU
style
86 # in testcases and the dg-* directives give too many false positives.
87 remove_testsuite ()
88 {
89 awk 'BEGIN{testsuite=0} /\+\+\+ / && ! /testsuite\//{testsuite=0} \
90 {if (!testsuite) print} /^\+\+\+
(.*\/)?testsuite\//{testsuite=1}'
91 }
92
93 grep $format '^+' $files \
94 | remove_testsuite \
95 | grep -v ':+++' \
96 > $inp
The /^\+\+\+ (.*\/)?testsuite\// doesn't ever match when the ^ anchor
is used.
The awk command matches fine by itself but not when fed from the "grep
$format '^+' $files"
command because grep adds the line numbers and file names.
So is it okay to omit the ^ here?
I think the AWK regex will not work correctly when the patch has
the line number prefix like "1234: " (AFAICT, this can only happen
in the second invocation of the remove_testsuite function which
also has the problem below making me wonder if your testing
exercised that mode).
Huh, you're right, but it didn't cause problems in my testing, which is weird.
I think the AWK regex needs to be changed to handle that. It should
start with something like "^([1-9][0-9]*:)?\+\+\+"
I think it needs to be
^(.*:)?([1-9][0-9]*:)?\+\+\+
because grep -nH would add the filename as well as the line number in the first
invocation of remove_testsuite.
This revision does that.
I tried to test the patch but it doesn't seem to work. When passed
a patch as an argument it hangs. The hunk below isn't quite right:
# Don't reuse $inp, which may be generated using -H and thus contain a
- # file prefix.
- grep -n '^+' $f \
+ # file prefix. Re-remove the testsuite since we're not using $inp.
+ remove_testsuite $f \
+ | grep -n '^+' \
| grep -v ':+++' \
> $tmp
The remove_testsuite function ignores arguments so passing $f to it
won't do anything except hang waiting for input. This should look
closer to this (it worked in my very limited testing):
cat $f | remove_testsuite \
Thanks for the help,
Kyrill
2016-10-13 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
* check_GNU_style.sh (remove_testsuite): New function.
Use it to remove testsuite from the diff.
Martin
diff --git a/contrib/check_GNU_style.sh b/contrib/check_GNU_style.sh
index 87a276c9cf47b5e07c4407f740ce05dce1928c30..fb7494661ee8ff4d4e58ed05137bb69aab7c46a7 100755
--- a/contrib/check_GNU_style.sh
+++ b/contrib/check_GNU_style.sh
@@ -81,7 +81,17 @@ if [ $nfiles -eq 1 ]; then
else
format="-nH"
fi
+
+# Remove the testsuite part of the diff. We don't care about GNU style
+# in testcases and the dg-* directives give too many false positives.
+remove_testsuite ()
+{
+ awk 'BEGIN{testsuite=0} /^(.*:)?([1-9][0-9]*:)?\+\+\+ / && ! /testsuite\//{testsuite=0} \
+ {if (!testsuite) print} /^(.*:)?([1-9][0-9]*:)?\+\+\+ (.*\/)?testsuite\//{testsuite=1}'
+}
+
grep $format '^+' $files \
+ | remove_testsuite \
| grep -v ':+++' \
> $inp
@@ -160,8 +170,9 @@ col (){
fi
# Don't reuse $inp, which may be generated using -H and thus contain a
- # file prefix.
- grep -n '^+' $f \
+ # file prefix. Re-remove the testsuite since we're not using $inp.
+ cat $f | remove_testsuite \
+ | grep -n '^+' \
| grep -v ':+++' \
> $tmp
@@ -174,11 +185,10 @@ col (){
# Expand tabs to spaces according to tab positions.
# Keep long lines, make short lines empty. Print the part past 80 chars
# in red.
- # Don't complain about dg-xxx directives in tests.
cat "$tmp" \
| sed 's/^[0-9]*:+//' \
| expand \
- | awk '$0 !~ /{[[:space:]]*dg-(error|warning|message)[[:space:]]/ { \
+ | awk '{ \
if (length($0) > 80) \
printf "%s\033[1;31m%s\033[0m\n", \
substr($0,1,80), \
