* [PATCH, 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
@ 2015-05-12 7:41 Tom de Vries
2015-05-12 15:18 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2015-05-12 7:41 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 198 bytes --]
Hi,
this patch fixes a problem in the 80 characters length check.
Currently tab width is not properly calculated.
The patch uses expand to interpret tabs properly.
OK for trunk?
Thanks,
- Tom
[-- Attachment #2: 0005-check_GNU_style.sh-Fix-tab-size-in-80-characters-che.patch --]
[-- Type: text/x-patch, Size: 1462 bytes --]
[PATCH 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
2015-05-11 Tom de Vries <tom@codesourcery.com>
* check_GNU_style.sh (col): Fix tab size.
---
contrib/check_GNU_style.sh | 38 +++++++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/contrib/check_GNU_style.sh b/contrib/check_GNU_style.sh
index 318eb6a..90c612f 100755
--- a/contrib/check_GNU_style.sh
+++ b/contrib/check_GNU_style.sh
@@ -116,13 +116,37 @@ vg (){
col (){
msg="$1"
- cat $inp \
- | awk -F':\\+' '{ if (length($2) > 80) print $0}' \
- > $tmp
- if [ -s $tmp ]; then
- printf "\n$msg\n"
- cat $tmp
- fi
+ local first=true
+ local f
+ for f in $files; do
+ local prefix=""
+ if [ $nfiles -ne 1 ]; then
+ prefix="$f:"
+ fi
+
+ # Don't reuse $inp, which may be generated using -H and thus contain a
+ # file prefix.
+ grep -n '^+' $f \
+ | grep -v ':+++' \
+ > $tmp
+
+ cat $tmp | while IFS= read -r line; do
+ local longline
+ # Filter out the line number prefix and the patch line modifier '+'
+ # to obtain the bare line, before we use expand.
+ longline=$(echo "$line" \
+ | sed 's/^[0-9]*:+//' \
+ | expand \
+ | awk '{ if (length($0) > 80) print $0}')
+ if [ "$longline" != "" ]; then
+ if $first; then
+ printf "\n$msg\n"
+ first=false
+ fi
+ echo "$prefix$line"
+ fi
+ done
+ done
}
col 'Lines should not exceed 80 characters.'
--
1.9.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
2015-05-12 7:41 [PATCH, 5/5] check_GNU_style.sh: Fix tab size in 80 characters check Tom de Vries
@ 2015-05-12 15:18 ` Jeff Law
2015-05-18 8:14 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2015-05-12 15:18 UTC (permalink / raw)
To: Tom de Vries, GCC Patches
On 05/12/2015 01:29 AM, Tom de Vries wrote:
> Hi,
>
> this patch fixes a problem in the 80 characters length check.
>
> Currently tab width is not properly calculated.
>
> The patch uses expand to interpret tabs properly.
>
> OK for trunk?
>
> Thanks,
> - Tom
>
>
> 0005-check_GNU_style.sh-Fix-tab-size-in-80-characters-che.patch
>
>
> [PATCH 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
>
> 2015-05-11 Tom de Vries<tom@codesourcery.com>
>
> * check_GNU_style.sh (col): Fix tab size.
OK.
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
2015-05-12 15:18 ` Jeff Law
@ 2015-05-18 8:14 ` Tom de Vries
2015-05-18 15:52 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2015-05-18 8:14 UTC (permalink / raw)
To: Jeff Law, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]
On 12-05-15 17:16, Jeff Law wrote:
>> [PATCH 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
>>
>> 2015-05-11 Tom de Vries<tom@codesourcery.com>
>>
>> * check_GNU_style.sh (col): Fix tab size.
> OK.
Hi Jeff,
I.
I noticed a performance degradation due to this patch:
...
$ cat gcc/tree-ssa-tail-merge.c | awk '{printf "+%s\n", $0}' | time -p
./contrib/check_GNU_style.sh -
...
real 4.10
user 0.71
sys 6.77
...
Before this patch, the 'real' time was roughly a factor 80 smaller:
...
real 0.05
user 0.02
sys 0.03
...
This degradation is due to the fact that the patch does the 80 chars check
line-by-line, and invokes processes for each new line.
II.
Attached follow-up patch rewrites the 80 chars check to handle a file at a time
rather than a line at a time, and gets performance back to normal:
...
real 0.07
user 0.03
sys 0.05
...
As a bonus, the bit longer than 80 chars is now printed in red, similar to how
the other checks show the output.
OK for trunk?
Thanks,
- Tom
[-- Attachment #2: 0001-check_GNU_style.sh-Don-t-do-80-char-check-line-by-li.patch --]
[-- Type: text/x-patch, Size: 3101 bytes --]
check_GNU_style.sh: Don't do 80 char check line by line
2015-05-18 Tom de Vries <tom@codesourcery.com>
* check_GNU_style.sh: Add temp files tmp2 and tmp3.
(cat_with_prefix): New function, using global variable prefix.
(col): Make prefix a global variable. Rewrite to process file at a time
rather than line at a time. Print part longer than 80 chars in red.
---
contrib/check_GNU_style.sh | 70 ++++++++++++++++++++++++++++++++++------------
1 file changed, 52 insertions(+), 18 deletions(-)
diff --git a/contrib/check_GNU_style.sh b/contrib/check_GNU_style.sh
index ab59b1e..033a2c9 100755
--- a/contrib/check_GNU_style.sh
+++ b/contrib/check_GNU_style.sh
@@ -65,10 +65,12 @@ fi
inp=check_GNU_style.inp
tmp=check_GNU_style.tmp
+tmp2=check_GNU_style.2.tmp
+tmp3=check_GNU_style.3.tmp
# Remove $tmp on exit and various signals.
-trap "rm -f $inp $tmp $stdin_tmp" 0
-trap "rm -f $inp $tmp $stdin_tmp; exit 1" 1 2 3 5 9 13 15
+trap "rm -f $inp $tmp $tmp2 $tmp3 $stdin_tmp" 0
+trap "rm -f $inp $tmp $tmp2 $tmp3 $stdin_tmp; exit 1" 1 2 3 5 9 13 15
if [ $nfiles -eq 1 ]; then
# There's no need for the file prefix if we're dealing only with one file.
@@ -80,6 +82,17 @@ grep $format '^+' $files \
| grep -v ':+++' \
> $inp
+cat_with_prefix ()
+{
+ local f="$1"
+
+ if [ "$prefix" = "" ]; then
+ cat "$f"
+ else
+ awk "{printf "%s%s\n", $prefix, \$0}" $f
+ fi
+}
+
# Grep
g (){
local msg="$1"
@@ -134,10 +147,11 @@ vg (){
col (){
local msg="$1"
+
local first=true
local f
for f in $files; do
- local prefix=""
+ prefix=""
if [ $nfiles -ne 1 ]; then
prefix="$f:"
fi
@@ -148,22 +162,42 @@ col (){
| grep -v ':+++' \
> $tmp
- cat $tmp | while IFS= read -r line; do
- local longline
- # Filter out the line number prefix and the patch line modifier '+'
- # to obtain the bare line, before we use expand.
- longline=$(echo "$line" \
- | sed 's/^[0-9]*:+//' \
- | expand \
- | awk '{ if (length($0) > 80) print $0}')
- if [ "$longline" != "" ]; then
- if $first; then
- printf "\n$msg\n"
- first=false
- fi
- echo "$prefix$line"
+ # Keep only line number prefix and patch modifier '+'.
+ cat "$tmp" \
+ | sed 's/\(^[0-9][0-9]*:+\).*/\1/' \
+ > "$tmp2"
+
+ # Remove line number prefix and patch modifier '+'.
+ # Expand tabs to spaces according to tab positions.
+ # Keep long lines, make short lines empty. Print the part past 80 chars
+ # in red.
+ cat "$tmp" \
+ | sed 's/^[0-9]*:+//' \
+ | expand \
+ | awk '{ \
+ if (length($0) > 80) \
+ printf "%s\033[1;31m%s\033[0m\n", \
+ substr($0,1,80), \
+ substr($0,81); \
+ else \
+ print "" \
+ }' \
+ > "$tmp3"
+
+ # Combine prefix back with long lines.
+ # Filter out empty lines.
+ local found=false
+ paste -d '' "$tmp2" "$tmp3" \
+ | grep -v '^[0-9][0-9]*:+$' \
+ > "$tmp" && found=true
+
+ if $found; then
+ if $first; then
+ printf "\n$msg\n"
+ first=false
fi
- done
+ cat_with_prefix "$tmp"
+ fi
done
}
--
1.9.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
2015-05-18 8:14 ` Tom de Vries
@ 2015-05-18 15:52 ` Jeff Law
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2015-05-18 15:52 UTC (permalink / raw)
To: Tom de Vries, GCC Patches
On 05/18/2015 02:07 AM, Tom de Vries wrote:
> On 12-05-15 17:16, Jeff Law wrote:
>>> [PATCH 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
>>>
>>> 2015-05-11 Tom de Vries<tom@codesourcery.com>
>>>
>>> * check_GNU_style.sh (col): Fix tab size.
>> OK.
>
> Hi Jeff,
>
> I.
>
> I noticed a performance degradation due to this patch:
> ...
> $ cat gcc/tree-ssa-tail-merge.c | awk '{printf "+%s\n", $0}' | time -p
> ./contrib/check_GNU_style.sh -
> ...
> real 4.10
> user 0.71
> sys 6.77
> ...
>
> Before this patch, the 'real' time was roughly a factor 80 smaller:
> ...
> real 0.05
> user 0.02
> sys 0.03
> ...
>
> This degradation is due to the fact that the patch does the 80 chars
> check line-by-line, and invokes processes for each new line.
>
>
> II.
>
> Attached follow-up patch rewrites the 80 chars check to handle a file at
> a time rather than a line at a time, and gets performance back to normal:
> ...
> real 0.07
> user 0.03
> sys 0.05
> ...
>
> As a bonus, the bit longer than 80 chars is now printed in red, similar
> to how the other checks show the output.
>
> OK for trunk?
Yes, this is fine.
jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-05-18 15:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 7:41 [PATCH, 5/5] check_GNU_style.sh: Fix tab size in 80 characters check Tom de Vries
2015-05-12 15:18 ` Jeff Law
2015-05-18 8:14 ` Tom de Vries
2015-05-18 15:52 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).