* [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh
@ 2023-03-24 11:46 Tom de Vries
2023-03-24 13:52 ` Simon Marchi
2023-03-24 14:16 ` Tom Tromey
0 siblings, 2 replies; 5+ messages in thread
From: Tom de Vries @ 2023-03-24 11:46 UTC (permalink / raw)
To: gdb-patches
There are a number of different ways to express the same thing in TCL.
For instance, $foo and ${foo}:
...
% set foo "foo bar"
foo bar
% puts ${foo}
foo bar
% puts $foo
foo bar
...
The braces make things (IMO) harder to read, but are necessary in some cases
though, for instance ${foo-bar} and $foo-bar are not the same thing:
...
% set foo "foo var"
foo var
% set foo-bar "foo-bar var"
foo-bar var
% puts ${foo-bar}
foo-bar var
% puts $foo-bar
foo var-bar
...
Furthermore, there's the tendency to use "$foo" (as is often necessary in shell
scripting), while in TCL using $foo is sufficient:
...
% set foo "bar bar"
bar bar
% puts "$foo"
bar bar
% puts $foo
bar bar
...
Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases
in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo",
where possible.
This patch doesn't contain the effects of running it, because that would make
the patch too large:
...
$ git diff | wc
63705 273146 2457187
...
If this approach is acceptable, I'll commit the effects as a seperate patch.
Tested on x86_64-linux.
---
gdb/contrib/testsuite-normalize.sh | 44 ++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100755 gdb/contrib/testsuite-normalize.sh
diff --git a/gdb/contrib/testsuite-normalize.sh b/gdb/contrib/testsuite-normalize.sh
new file mode 100755
index 00000000000..0cbde3526ce
--- /dev/null
+++ b/gdb/contrib/testsuite-normalize.sh
@@ -0,0 +1,44 @@
+#!/bin/bash
+
+handle_file ()
+{
+ f="$1"
+
+ # Rewrite '${foo}' -> '$foo''.
+ # Don't rewrite '\${foo}' -> '\$foo'.
+ # Don't rewrite '${foo}(bar)' -> '$foo(bar).
+ # Don't rewrite '${foo}bar' -> '$foobar'.
+ # Don't rewrite '${foo}::bar' -> '$foo::bar'.
+ # Two variants, first one matching '${foo}<eol>'.
+ sed -i 's/\([^\\]\)${\([a-zA-Z0-9_]*\)}$/\1$\2/g' "$f"
+ sed -i 's/\([^\\]\)${\([a-zA-Z0-9_]*\)}\([^a-zA-Z0-9_(:]\)/\1$\2\3/g' "$f"
+
+ # Rewrite ' "$foo" ' -> ' $foo '.
+ # Rewrite ' "$foo"<eol>' -> ' $foo<eol>'.
+ sed -i 's/\([ \t][ \t]*\)"\$\([a-zA-Z0-9_]*\)"$/\1$\2/g' "$f"
+ sed -i 's/\([ \t][ \t]*\)"\$\([a-zA-Z0-9_]*\)"\([ \t][ \t]*\)/\1$\2\3/g' "$f"
+}
+
+main ()
+{
+ if [ $# -eq 0 ]; then
+ mapfile files < <(find gdb/testsuite -type f -name "*.exp*")
+ else
+ files=("$@")
+ fi
+
+ for f in "${files[@]}"; do
+ f=$(echo $f)
+ sum=$(md5sum "$f")
+ while true; do
+ handle_file "$f"
+ prev=$sum
+ sum=$(md5sum "$f")
+ if [ "$sum" == "$prev" ]; then
+ break
+ fi
+ done
+ done
+}
+
+main "$@"
base-commit: ca357a9359f5d09058d27fc1f84f1d53c2717ba1
--
2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh 2023-03-24 11:46 [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh Tom de Vries @ 2023-03-24 13:52 ` Simon Marchi 2023-03-24 14:39 ` Tom de Vries 2023-03-24 14:16 ` Tom Tromey 1 sibling, 1 reply; 5+ messages in thread From: Simon Marchi @ 2023-03-24 13:52 UTC (permalink / raw) To: Tom de Vries, gdb-patches On 3/24/23 07:46, Tom de Vries via Gdb-patches wrote: > There are a number of different ways to express the same thing in TCL. > > For instance, $foo and ${foo}: > ... > % set foo "foo bar" > foo bar > % puts ${foo} > foo bar > % puts $foo > foo bar > ... > > The braces make things (IMO) harder to read, but are necessary in some cases > though, for instance ${foo-bar} and $foo-bar are not the same thing: > ... > % set foo "foo var" > foo var > % set foo-bar "foo-bar var" > foo-bar var > % puts ${foo-bar} > foo-bar var > % puts $foo-bar > foo var-bar > ... > > Furthermore, there's the tendency to use "$foo" (as is often necessary in shell > scripting), while in TCL using $foo is sufficient: > ... > % set foo "bar bar" > bar bar > % puts "$foo" > bar bar > % puts $foo > bar bar > ... > > Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases > in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", > where possible. > > This patch doesn't contain the effects of running it, because that would make > the patch too large: > ... > $ git diff | wc > 63705 273146 2457187 > ... > > If this approach is acceptable, I'll commit the effects as a seperate patch. I have nothing against it. I like when the formatting is normalized (which is why is <3 black for Python). > Tested on x86_64-linux. ➜ gdb git:(master) ✗ pwd /home/smarchi/src/binutils-gdb/gdb ➜ gdb git:(master) ✗ ./contrib/testsuite-normalize.sh find: ‘gdb/testsuite’: No such file or directory My CWD is the gdb directory 99% of the time. IWBN if the script would work regardless of where you are in the source repo (it looks like you need to be in the root of the repo for it to work). Perhaps it could locate the root directory of the git repository and cd there? I tried it and looked at a bunch of files, the changes seemed fine. > +main () > +{ > + if [ $# -eq 0 ]; then > + mapfile files < <(find gdb/testsuite -type f -name "*.exp*") > + else > + files=("$@") > + fi > + > + for f in "${files[@]}"; do > + f=$(echo $f) What's the purpose of this echo? shellcheck says: In gdb/contrib/testsuite-normalize.sh line 31: f=$(echo $f) ^--------^ SC2116 (style): Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. ^-- SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: f=$(echo "$f") Simon ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh 2023-03-24 13:52 ` Simon Marchi @ 2023-03-24 14:39 ` Tom de Vries 0 siblings, 0 replies; 5+ messages in thread From: Tom de Vries @ 2023-03-24 14:39 UTC (permalink / raw) To: Simon Marchi, gdb-patches [-- Attachment #1: Type: text/plain, Size: 2958 bytes --] On 3/24/23 14:52, Simon Marchi wrote: > On 3/24/23 07:46, Tom de Vries via Gdb-patches wrote: >> There are a number of different ways to express the same thing in TCL. >> >> For instance, $foo and ${foo}: >> ... >> % set foo "foo bar" >> foo bar >> % puts ${foo} >> foo bar >> % puts $foo >> foo bar >> ... >> >> The braces make things (IMO) harder to read, but are necessary in some cases >> though, for instance ${foo-bar} and $foo-bar are not the same thing: >> ... >> % set foo "foo var" >> foo var >> % set foo-bar "foo-bar var" >> foo-bar var >> % puts ${foo-bar} >> foo-bar var >> % puts $foo-bar >> foo var-bar >> ... >> >> Furthermore, there's the tendency to use "$foo" (as is often necessary in shell >> scripting), while in TCL using $foo is sufficient: >> ... >> % set foo "bar bar" >> bar bar >> % puts "$foo" >> bar bar >> % puts $foo >> bar bar >> ... >> >> Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases >> in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", >> where possible. >> >> This patch doesn't contain the effects of running it, because that would make >> the patch too large: >> ... >> $ git diff | wc >> 63705 273146 2457187 >> ... >> >> If this approach is acceptable, I'll commit the effects as a seperate patch. > > I have nothing against it. I like when the formatting is normalized > (which is why is <3 black for Python). > Yeah, I wish there was an existing tool we could use, but I haven't found anything. >> Tested on x86_64-linux. > > ➜ gdb git:(master) ✗ pwd > /home/smarchi/src/binutils-gdb/gdb > ➜ gdb git:(master) ✗ ./contrib/testsuite-normalize.sh > find: ‘gdb/testsuite’: No such file or directory > > My CWD is the gdb directory 99% of the time. IWBN if the script would > work regardless of where you are in the source repo (it looks like you > need to be in the root of the repo for it to work). Perhaps it could > locate the root directory of the git repository and cd there? > Ack, fixed (FWIW, not using cd though). > I tried it and looked at a bunch of files, the changes seemed fine. > >> +main () >> +{ >> + if [ $# -eq 0 ]; then >> + mapfile files < <(find gdb/testsuite -type f -name "*.exp*") >> + else >> + files=("$@") >> + fi >> + >> + for f in "${files[@]}"; do >> + f=$(echo $f) > > What's the purpose of this echo? Strip trailing newline. > shellcheck says: > > > In gdb/contrib/testsuite-normalize.sh line 31: > f=$(echo $f) > ^--------^ SC2116 (style): Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. > ^-- SC2086 (info): Double quote to prevent globbing and word splitting. > > Did you mean: > f=$(echo "$f") > > I've managed to fix this by using instead: ... f=${f%$'\n'} ... It passes shellcheck now. Also added copyright notice, as requested by Tom in the other reply. Thanks, - Tom [-- Attachment #2: 0001-gdb-testsuite-Add-gdb-contrib-testsuite-normalize.sh.patch --] [-- Type: text/x-patch, Size: 3729 bytes --] From e593407376bb55dbd8e5484098161f3cc6ffd4cf Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Sat, 4 Mar 2023 13:40:24 +0100 Subject: [PATCH] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh There are a number of different ways to express the same thing in TCL. For instance, $foo and ${foo}: ... % set foo "foo bar" foo bar % puts ${foo} foo bar % puts $foo foo bar ... The braces make things (IMO) harder to read, but are necessary in some cases though, for instance ${foo-bar} and $foo-bar are not the same thing: ... % set foo "foo var" foo var % set foo-bar "foo-bar var" foo-bar var % puts ${foo-bar} foo-bar var % puts $foo-bar foo var-bar ... Furthermore, there's the tendency to use "$foo" (as is often necessary in shell scripting), while in TCL using $foo is sufficient: ... % set foo "bar bar" bar bar % puts "$foo" bar bar % puts $foo bar bar ... Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", where possible. This patch doesn't contain the effects of running it, because that would make the patch too large: ... $ git diff | wc 63705 273146 2457187 ... If this approach is acceptable, I'll commit the effects as a seperate patch. Tested on x86_64-linux. --- gdb/contrib/testsuite-normalize.sh | 63 ++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100755 gdb/contrib/testsuite-normalize.sh diff --git a/gdb/contrib/testsuite-normalize.sh b/gdb/contrib/testsuite-normalize.sh new file mode 100755 index 00000000000..be5f89368ee --- /dev/null +++ b/gdb/contrib/testsuite-normalize.sh @@ -0,0 +1,63 @@ +#!/usr/bin/env bash + +# Copyright (C) 2023 Free Software Foundation, Inc. +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +gdb_contrib_dir=$(cd "$(dirname "$0")" || exit; pwd -P) +gdb_testsuite_dir=$gdb_contrib_dir/../testsuite + +handle_file () +{ + f="$1" + + # Rewrite '${foo}' -> '$foo''. + # Don't rewrite '\${foo}' -> '\$foo'. + # Don't rewrite '${foo}(bar)' -> '$foo(bar). + # Don't rewrite '${foo}bar' -> '$foobar'. + # Don't rewrite '${foo}::bar' -> '$foo::bar'. + # Two variants, first one matching '${foo}<eol>'. + sed -i 's/\([^\\]\)${\([a-zA-Z0-9_]*\)}$/\1$\2/g' "$f" + sed -i 's/\([^\\]\)${\([a-zA-Z0-9_]*\)}\([^a-zA-Z0-9_(:]\)/\1$\2\3/g' "$f" + + # Rewrite ' "$foo" ' -> ' $foo '. + # Rewrite ' "$foo"<eol>' -> ' $foo<eol>'. + sed -i 's/\([ \t][ \t]*\)"\$\([a-zA-Z0-9_]*\)"$/\1$\2/g' "$f" + sed -i 's/\([ \t][ \t]*\)"\$\([a-zA-Z0-9_]*\)"\([ \t][ \t]*\)/\1$\2\3/g' "$f" +} + +main () +{ + if [ $# -eq 0 ]; then + mapfile files < <(find "$gdb_testsuite_dir" -type f -name "*.exp*") + else + files=("$@") + fi + + for f in "${files[@]}"; do + # Strip trailing newline. + f=${f%$'\n'} + + sum=$(md5sum "$f") + while true; do + handle_file "$f" + prev=$sum + sum=$(md5sum "$f") + if [ "$sum" == "$prev" ]; then + break + fi + done + done +} + +main "$@" base-commit: 4460691252d5c345f0b34ac366639df23c687832 -- 2.35.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh 2023-03-24 11:46 [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh Tom de Vries 2023-03-24 13:52 ` Simon Marchi @ 2023-03-24 14:16 ` Tom Tromey 2023-03-24 14:42 ` Tom de Vries 1 sibling, 1 reply; 5+ messages in thread From: Tom Tromey @ 2023-03-24 14:16 UTC (permalink / raw) To: Tom de Vries via Gdb-patches; +Cc: Tom de Vries >>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes: Tom> Furthermore, there's the tendency to use "$foo" (as is often necessary in shell Tom> scripting), while in TCL using $foo is sufficient: FWIW I sometimes find myself using unnecessary quotes in Tcl just to make some things highlight differently in Emacs. So I'd imagine there's a lot more quoting that could be removed if we cared to. Tom> Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases Tom> in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", Tom> where possible. The idea seems fine to me. I didn't really read the code. Tom> +++ b/gdb/contrib/testsuite-normalize.sh Tom> @@ -0,0 +1,44 @@ Tom> +#!/bin/bash Tom> + Should have a copyright header. Tom ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh 2023-03-24 14:16 ` Tom Tromey @ 2023-03-24 14:42 ` Tom de Vries 0 siblings, 0 replies; 5+ messages in thread From: Tom de Vries @ 2023-03-24 14:42 UTC (permalink / raw) To: Tom Tromey, Tom de Vries via Gdb-patches On 3/24/23 15:16, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes: > > Tom> Furthermore, there's the tendency to use "$foo" (as is often necessary in shell > Tom> scripting), while in TCL using $foo is sufficient: > > FWIW I sometimes find myself using unnecessary quotes in Tcl just to > make some things highlight differently in Emacs. So I'd imagine there's > a lot more quoting that could be removed if we cared to. > > Tom> Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases > Tom> in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", > Tom> where possible. > > The idea seems fine to me. Great, thanks. > I didn't really read the code. > > Tom> +++ b/gdb/contrib/testsuite-normalize.sh > Tom> @@ -0,0 +1,44 @@ > Tom> +#!/bin/bash > Tom> + > > Should have a copyright header. Ack, fixed in this ( https://sourceware.org/pipermail/gdb-patches/2023-March/198278.html ) update. Thanks, - Tom ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-24 14:42 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-24 11:46 [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh Tom de Vries 2023-03-24 13:52 ` Simon Marchi 2023-03-24 14:39 ` Tom de Vries 2023-03-24 14:16 ` Tom Tromey 2023-03-24 14:42 ` Tom de Vries
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).