* [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).