Contribution by Patrick Fradin <patrick.fradin@planar.com>contrib_patrick_fradin_code_cleanup

Quoting his mail:

<pre>
==========================================================
Hi Jeff,

I spent some of my time to contribute to the TclTk community ! I'm in
late for Christmas gift but like we said in French : "Mieux vaut tard
que jamais". ;-)

I've use TclDevKit 5.3.0 tclchecker to analyse TclTk code in Tcl and
Tk library directories (library, tools and tests) to correct a lot of
warnings and few errors. (encapsulate some expr, use 'chan xxx'
instead of fconfigure, fileevent...)

I've made some improvements too :

Examples :

- Use 'lassign' instead of many 'lindex' of 'foreach/break' loop.

- Use 'in' or 'ni' operators instead of 'lsearch -exact' or to
  factorise some eq/ne && / || tests.

- Use 'eq' or 'ne' to tests strings instead of '==' or '!='.

- Use 'unset -nocomplain' to avoid 'catch {unset...}'.

- Remove some useless catch around 'destroy' calls.

- Use expand {*} instead of 'eval'. Don't touch a lot of code because
  I don't know all structs and lists. I think it could be a greater
  improvement to reduce 'eval' calls.

Due to previous experience, I dot not change any indentation ! ;-)
==========================================================
</pre>

17 files changed, 93 insertions, 64 deletions

diff --git a/tests/ttk/checkbutton.test b/tests/ttk/checkbutton.test
index e18ff32..ec58173 100644
--- a/tests/ttk/checkbutton.test
+++ b/tests/ttk/checkbutton.test
@@ -3,8 +3,9 @@
 #
 
 package require Tk
-package require tcltest ; namespace import -force tcltest::*
-loadTestedCommands
+package require tcltest
+namespace import -force tcltest::*
+tcltest::loadTestedCommands 
 
 test checkbutton-1.1 "Checkbutton check" -body {
     pack [ttk::checkbutton .cb -text "TCheckbutton" -variable cb]
@@ -43,6 +44,6 @@ test checkbutton-1.6 "Checkbutton default variable" -body {
     lappend result [info exists .cb] [set .cb] [.cb state]
     .cb invoke
     lappend result [info exists .cb] [set .cb] [.cb state]
-} -result [list .cb 0 alternate   1 on selected  1 off {}]
+} -result [list .cb 0 alternate   1 on selected  1 off ""]
 
 tcltest::cleanupTests
diff --git a/tests/ttk/combobox.test b/tests/ttk/combobox.test
index 43f3cf1..28eb459 100644
--- a/tests/ttk/combobox.test
+++ b/tests/ttk/combobox.test
@@ -3,8 +3,9 @@
 #
 
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
-loadTestedCommands
+package require tcltest
+namespace import -force tcltest::*
+::tcltest::loadTestedCommands
 
 test combobox-1.0 "Combobox tests -- setup" -body {
     ttk::combobox .cb
@@ -45,7 +46,6 @@ test combobox-2.4 "current -- value not in list" -body {
 
 test combobox-2.end "Cleanup" -body { destroy .cb }
 
-
 test combobox-1890211 "ComboboxSelected event after listbox unposted" -body {
     # whitebox test...
     pack [ttk::combobox .cb -values [list a b c]]
@@ -61,7 +61,7 @@ test combobox-1890211 "ComboboxSelected event after listbox unposted" -body {
     lappend result Select [winfo ismapped .cb.popdown] [.cb get]
     update
     set result
-} -result [list Start 0 {} Post 1 {} Select 0 b Event 0 b] -cleanup {
+} -result [list Start 0 "" Post 1 "" Select 0 b Event 0 b] -cleanup {
     destroy .cb
 }
 
diff --git a/tests/ttk/entry.test b/tests/ttk/entry.test
index 0c2f0be..25e8194 100644
--- a/tests/ttk/entry.test
+++ b/tests/ttk/entry.test
@@ -3,11 +3,12 @@
 #
 
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 variable scrollInfo
-proc scroll args {
+proc scroll {args} {
     global scrollInfo
     set scrollInfo $args
 }
@@ -17,9 +18,10 @@ proc scroll args {
 #
 variable bgerror ""
 proc bgerror {error} {
+    global errorInfo errorCode
     variable bgerror $error
-    variable bgerrorInfo $::errorInfo
-    variable bgerrorCode $::errorCode
+    variable bgerrorInfo $errorInfo
+    variable bgerrorCode $errorCode
 }
 
 #
@@ -96,6 +98,7 @@ test entry-3.0 "Series 3 setup" -body {
 }
 
 test entry-3.1 "bbox widget command" -body {
+    variable bd ch
     .e delete 0 end
     .e bbox 0
 } -result [list $bd $bd 0 $ch]
@@ -190,7 +193,7 @@ test entry-6.1 {Update linked variable in write trace} -body {
 	global x
 	set x "Overridden!"
     }
-    catch {destroy .e}
+    destroy .e
     set x ""
     trace variable x w override
     ttk::entry .e -textvariable x
diff --git a/tests/ttk/image.test b/tests/ttk/image.test
index a55f7f8..f239b8f 100644
--- a/tests/ttk/image.test
+++ b/tests/ttk/image.test
@@ -1,5 +1,6 @@
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 test image-1.1 "Bad image element" -body {
diff --git a/tests/ttk/labelframe.test b/tests/ttk/labelframe.test
index 28b4d2e..c095853 100644
--- a/tests/ttk/labelframe.test
+++ b/tests/ttk/labelframe.test
@@ -1,5 +1,6 @@
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 test labelframe-1.0 "Setup" -body {
diff --git a/tests/ttk/layout.test b/tests/ttk/layout.test
index 814e1d9..227246a 100644
--- a/tests/ttk/layout.test
+++ b/tests/ttk/layout.test
@@ -1,5 +1,6 @@
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 test layout-1.1 "Size computations for mixed-orientation layouts" -body {
@@ -21,5 +22,4 @@ test layout-1.1 "Size computations for mixed-orientation layouts" -body {
 
 } -cleanup { destroy .b } -result [list 24 24]
 
-
 tcltest::cleanupTests
diff --git a/tests/ttk/notebook.test b/tests/ttk/notebook.test
index cdce020..7b9a2dc 100644
--- a/tests/ttk/notebook.test
+++ b/tests/ttk/notebook.test
@@ -1,5 +1,6 @@
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 test notebook-1.0 "Setup" -body {
@@ -405,7 +406,7 @@ test notebook-7.8a "move tabs - current tab undisturbed - exhaustive" -body {
 	    foreach k {0 1 2 3 4} {
 		.nb insert $j $k
 		set current [lindex [.nb tabs] [.nb index current]]
-		if {$current != ".nb.f$i"} {
+		if {$current ne ".nb.f$i"} {
 		    error "($i,$j,$k) current = $current"
 		}
 		.nb insert $k $j
@@ -425,7 +426,7 @@ test notebook-7.8b "insert new - current tab undisturbed - exhaustive" -body {
 .nb select .nb.f$i
 	    .nb insert $j [frame .nb.newf]
 	    set current [lindex [.nb tabs] [.nb index current]]
-	    if {$current != ".nb.f$i"} {
+	    if {$current ne ".nb.f$i"} {
 		puts stderr "new tab at $j, current = $current, expect .nb.f$i"
 	    }
 	    destroy .nb.newf
diff --git a/tests/ttk/panedwindow.test b/tests/ttk/panedwindow.test
index 7fe5c87..3fbeea1 100644
--- a/tests/ttk/panedwindow.test
+++ b/tests/ttk/panedwindow.test
@@ -1,5 +1,6 @@
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 proc propagate-geometry {} { update idletasks }
@@ -88,7 +89,7 @@ test panedwindow-2.2 "..., cont'd" -body {
     set w3 [winfo width .]
     set rw3 [winfo reqwidth .pw]
 
-    expr {$w3 == $w2 && $rw3 < $rw2}
+    expr {($w3 == $w2) && ($rw3 < $rw2)}
     # problem: [winfo reqwidth] shrinks, but sashes haven't moved
     # since we haven't gotten a ConfigureNotify.
     # How to (a) check for this, and (b) fix it?
@@ -124,10 +125,8 @@ test panedwindow-3.2 "add pane -- errors" -body {
     .pw add [ttk::label .pw.l] -weight -1
 } -returnCodes 1 -match glob -result "-weight must be nonnegative"
 
-
 test panedwindow-3.end "cleanup" -body { destroy .pw }
 
-
 test panedwindow-4.1 "forget" -body {
     pack [ttk::panedwindow .pw -orient vertical] -expand true -fill both
     .pw add [label .pw.l1 -text "L1"]
@@ -201,7 +200,7 @@ test panedwindow-5.1 "Propagate Map/Unmap state to children" -body {
 proc sashpositions {pw} {
     set positions [list]
     set npanes [llength [winfo children $pw]]
-    for {set i 0} {$i < $npanes - 1} {incr i} {
+    for {set i 0} {$i < ($npanes - 1)} {incr i} {
     	lappend positions [$pw sashpos $i]
     }
     return $positions
@@ -219,7 +218,7 @@ test paned-sashpos-setup "Setup for sash position test" -body {
 
     propagate-geometry
     list [winfo reqwidth .pw] [winfo reqheight .pw]
-} -result [list 20 [expr {20*4 + 5*3}]]
+} -result [list 20 [expr {(20 * 4) + (5 * 3)}]]
 
 test paned-sashpos-attempt-restore "Attempt to set sash positions" -body {
     # This is not expected to succeed, since .pw isn't large enough yet.
diff --git a/tests/ttk/progressbar.test b/tests/ttk/progressbar.test
index b9add86..98ce72d 100644
--- a/tests/ttk/progressbar.test
+++ b/tests/ttk/progressbar.test
@@ -1,8 +1,8 @@
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force ::tcltest::*
 loadTestedCommands
 
-
 test progressbar-1.1 "Setup" -body {
     ttk::progressbar .pb
 } -result .pb
diff --git a/tests/ttk/radiobutton.test b/tests/ttk/radiobutton.test
index ba02954..397602b 100644
--- a/tests/ttk/radiobutton.test
+++ b/tests/ttk/radiobutton.test
@@ -3,7 +3,8 @@
 #
 
 package require Tk
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 test radiobutton-1.1 "Radiobutton check" -body {
diff --git a/tests/ttk/scrollbar.test b/tests/ttk/scrollbar.test
index 0464273..3a2e17b 100644
--- a/tests/ttk/scrollbar.test
+++ b/tests/ttk/scrollbar.test
@@ -1,5 +1,6 @@
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 testConstraint coreScrollbar [expr {[tk windowingsystem] eq "aqua"}]
diff --git a/tests/ttk/spinbox.test b/tests/ttk/spinbox.test
index 3397e37..93290ec 100644
--- a/tests/ttk/spinbox.test
+++ b/tests/ttk/spinbox.test
@@ -3,7 +3,8 @@
 #
 
 package require Tk
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 test spinbox-1.0 "Spinbox tests -- setup" -body {
@@ -54,7 +55,6 @@ test spinbox-1.4.2 "set changes value" -setup {
     destroy .sb
 } -result 33
 
-
 test spinbox-1.6.1 "insert start" -setup {
     ttk::spinbox .sb -from 0 -to 100
 } -body {
@@ -150,7 +150,6 @@ test spinbox-1.8.4 "-validate option: " -setup {
     destroy .sb
 } -result {50}
 
-
 test spinbox-2.0 "current command -- unset should be 0" -constraints nyi -setup {
     ttk::spinbox .sb -values [list a b c d e a]
 } -body {
diff --git a/tests/ttk/treetags.test b/tests/ttk/treetags.test
index 7f26e2f..e9ca8d1 100644
--- a/tests/ttk/treetags.test
+++ b/tests/ttk/treetags.test
@@ -1,6 +1,7 @@
 
 package require Tk
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 ### treeview tag invariants:
diff --git a/tests/ttk/treeview.test b/tests/ttk/treeview.test
index aa7e64a..9372e3f 100644
--- a/tests/ttk/treeview.test
+++ b/tests/ttk/treeview.test
@@ -4,7 +4,8 @@
 #
 
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 # consistencyCheck --
@@ -14,7 +15,7 @@ loadTestedCommands
 #	Since [$tv children] follows ->next links and [$tv index]
 #	follows ->prev links, this should cover all invariants.
 #
-proc consistencyCheck {tv {item {}}} {
+proc consistencyCheck {tv {item ""}} {
     set i 0;
     foreach child [$tv children $item] {
 	assert {[$tv parent $child] == $item} "parent $child = $item"
@@ -334,7 +335,6 @@ test treeview-5.13 "get, no value" -body {
     set result
 } -result {}
 
-
 test treeview-6.1 "deletion - setup" -body {
     .tv insert {} end -id dtest
     foreach id [list a b c d e] {
@@ -462,13 +462,15 @@ test treeview-8.5 "Selection - bad operation" -body {
 ### NEED: more tests for see/yview/scrolling
 
 proc scrollcallback {args} {
-    set ::scrolldata $args
+    global scrolldata
+    set scrolldata $args
 }
 test treeview-9.0 "scroll callback - empty tree" -body {
+    global scrolldata
     .tv configure -yscrollcommand scrollcallback
     .tv delete [.tv children {}]
     update
-    set ::scrolldata
+    set scrolldata
 } -result [list 0.0 1.0]
 
 ### identify tests:
diff --git a/tests/ttk/ttk.test b/tests/ttk/ttk.test
index e58b021..1332338 100644
--- a/tests/ttk/ttk.test
+++ b/tests/ttk/ttk.test
@@ -1,9 +1,10 @@
 
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
-proc skip args {}
+proc skip {args} {}
 proc ok {} { return }
 
 variable widgetClasses {
@@ -15,9 +16,10 @@ variable widgetClasses {
 }
 
 proc bgerror {error} {
+    global errorInfo errorCode
     variable bgerror $error
-    variable bgerrorInfo $::errorInfo
-    variable bgerrorCode $::errorCode
+    variable bgerrorInfo $errorInfo
+    variable bgerrorCode $errorCode
 }
 
 # Self-destruct tests.
@@ -226,7 +228,7 @@ foreach wc $widgetClasses {
 	    .w cget $option
 	}
     } -cleanup {
-	catch {destroy .w}
+	destroy .w
     }
 }
 
@@ -245,7 +247,8 @@ test ttk-3.2 "Propagate errors from variable traces" -body {
     ttk::checkbutton .cb -variable A
     .cb invoke
 } -cleanup {
-    unset ::A ; destroy .cb
+    unset ::A
+    destroy .cb
 } -returnCodes error -result {can't set "A": failure}
 
 test ttk-3.3 "Constructor failure with cursor" -body {
@@ -267,7 +270,7 @@ test ttk-3.4 "SF#2009213" -body {
 #
 
 test ttk-4.0 "Setup" -body {
-    catch { destroy .t }
+    destroy .t
     pack [ttk::label .t -text "Button 1"]
     testConstraint fontOption [expr ![catch { set prevFont [.t cget -font] }]]
     ok
@@ -317,17 +320,28 @@ zCDoBooud2PMDIKuRqDocgtGzMwg6O4Eii5z4Kgi6DIMhqLoagQGjiqCLvPgYOgqji6CLrfi
 variable compoundStrings {text image center top bottom left right none}
 
 if {0} {
-    proc now {} { set ::now [clock clicks -milliseconds] }
-    proc tick {} { puts -nonewline stderr "+" ; flush stderr }
+    proc now {} {
+        global now
+        set now [clock milliseconds]
+    }
+    proc tick {} {
+        puts -nonewline stderr "+"
+        flush stderr
+    }
     proc tock {} {
-	set then $::now; set ::now [clock clicks -milliseconds]
-	puts stderr " [expr {$::now - $then}] ms"
+        global now
+	set then $now
+        set now [clock milliseconds]
+	puts stderr " [expr {$now - $then}] ms"
     }
 } else {
-    proc now {} {} ; proc tick {} {} ; proc tock {} {}
+    proc now {} {}
+    proc tick {} {}
+    proc tock {} {}
 }
 
-now ; tick
+now 
+tick 
 test ttk-8.0 "Setup for 8.X" -body {
     ttk::button .ctb
     image create photo icon -data $::iconData;
@@ -335,7 +349,7 @@ test ttk-8.0 "Setup for 8.X" -body {
 }
 tock
 
-now
+now 
 test ttk-8.1 "Test -compound options" -body {
     # Exhaustively test each combination.
     # Main goal is to make sure no code paths crash.
@@ -343,12 +357,13 @@ test ttk-8.1 "Test -compound options" -body {
         foreach text {"Hi!" ""} {
 	    foreach compound $::compoundStrings {
 		.ctb configure -image $image -text $text -compound $compound
-		update; tick
+		update
+	        tick 
 	    }
 	}
     }
 }
-tock
+tock 
 
 test ttk-8.2 "Test -compound options with regular button" -body {
     button .rtb
@@ -358,24 +373,26 @@ test ttk-8.2 "Test -compound options with regular button" -body {
         foreach text {"Hi!" ""} {
 	    foreach compound [lrange $::compoundStrings 2 end] {
 		.rtb configure -image $image -text $text -compound $compound
-		update; tick
+		update
+	        tick 
 	    }
 	}
     }
 }
-tock
+tock 
 
 test ttk-8.3 "Rerun test 8.1" -body {
     foreach image {icon ""} {
         foreach text {"Hi!" ""} {
 	    foreach compound $::compoundStrings {
 		.ctb configure -image $image -text $text -compound $compound
-		update; tick
+		update
+	        tick 
 	    }
 	}
     }
 }
-tock
+tock 
 
 test ttk-8.4 "ImageChanged" -body {
     ttk::button .b -image icon
@@ -425,9 +442,11 @@ test ttk-9.7 "Unset textvariable, comparison" -body {
 # NB: this is on purpose: I believe the standard behaviour is the Wrong Thing
 #
     unset -nocomplain V1  V2
-    label .l -text Foo ; ttk::label .tl -text Foo
+    label .l -text Foo
+    ttk::label .tl -text Foo
 
-    .l configure -textvariable V1 ; .tl configure -textvariable V2
+    .l configure -textvariable V1
+    .tl configure -textvariable V2
     list [set V1] [info exists V2]
 } -cleanup { destroy .l .tl } -result [list Foo 0]
 
diff --git a/tests/ttk/validate.test b/tests/ttk/validate.test
index 417deac..4d9d5ca 100644
--- a/tests/ttk/validate.test
+++ b/tests/ttk/validate.test
@@ -22,8 +22,7 @@ test validate-0.0 "Setup" -constraints ttkEntry -body {
 
 test validate-0.1 "More setup" -body {
     destroy .e
-    catch {unset ::e}
-    catch {unset ::vVals}
+    unset -nocomplain ::e ::vVals
     entry .e -validate all \
 	    -validatecommand [list doval %W %d %i %P %s %S %v %V] \
 	    -invalidcommand bell \
@@ -209,7 +208,7 @@ test validate-2.1 "Validation script changes value" -body {
 # DIFFERENCE: core entry disables validation, ttk entry does not.
 
 destroy .e
-catch {unset ::e ::vVals}
+unset -nocomplain ::e ::vVals
 
 # See bug #1236979
 
diff --git a/tests/ttk/vsapi.test b/tests/ttk/vsapi.test
index bb88fef..450787b 100644
--- a/tests/ttk/vsapi.test
+++ b/tests/ttk/vsapi.test
@@ -2,7 +2,8 @@
 #
 
 package require Tk 8.5
-package require tcltest ; namespace import -force tcltest::*
+package require tcltest
+namespace import -force tcltest::*
 loadTestedCommands
 
 testConstraint xpnative \