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