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.tkachov@arm.com>
* check_GNU_style.sh (remove_testsuite): New function.
Use it to remove testsuite from the diff.
Martin