{"thread":{"id":"37918","subject":"[PATCH v4 0/2] gitk: save only changed configuration on exit","startedAt":"2014-11-09T22:20:00Z","lastAt":"2015-03-02T20:43:54Z","messageCount":7,"participants":["Max Kirillov","Paul Mackerras"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"251639","messageId":"1415571602-5858-1-git-send-email-max@max630.net","threadId":"37918","inReplyTo":null,"subject":"[PATCH v4 0/2] gitk: save only changed configuration on exit","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-11-09T22:20:00Z","receivedAt":"2014-11-09T22:20:00Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"v3 did not actually work for views.\n\nFix it (add global) and also set viewchanged in delview\n\nMax Kirillov (2):\n  gitk: write only changed configuration variables\n  gitk: synchronize config write\n\n gitk | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------\n 1 file changed, 107 insertions(+), 13 deletions(-)\n\n-- \n2.0.1.1697.g73c6810\n"},{"id":"251640","messageId":"1415571602-5858-2-git-send-email-max@max630.net","threadId":"37918","inReplyTo":"1415571602-5858-1-git-send-email-max@max630.net","subject":"[PATCH v4 1/2] gitk: write only changed configuration variables","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-11-09T22:20:01Z","receivedAt":"2014-11-09T22:20:01Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"When gitk contains some changed parameter, and there is existing\ninstance of gitk where the parameter is still old, it is reverted to\nthat old value when the instance exits.\n\nInstead, store a parameter in config only it is has been modified in the\nexiting instance. Otherwise, preserve the value which currently is in\nfile.  This allows editing the configuration when several instances are\nrunning, and don't get rollback of the modification if some other\ninstance where the configuration was not edited is closed last.\n\nFor scalar variables, use trace(3tcl) to detect their change. Since `trace` can\nsend bogus events, doublecheck if the value has really been changed, but once\nit is marked as changed, do not reset it back to unchanged ever, because if\nuser has restored the original value, it's the decision which should be stored\nas well as modified value.\n\nTreat view list especially: instead of rewriting the whole list, merge\nindividual views. Place old and updated views at their older placed, add\nnew ones to the end of list. Collect modified view explicitly, in newviewok{}\nand delview{}.\n\nDo not merge geometry values. They are almost always changing because\nuser moves and resises windows, and there is no way to find which one of\nthe geometries is most desired. Just overwrite them unconditionally,\nlike earlier.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\n gitk | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------\n 1 file changed, 77 insertions(+), 10 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 78358a7..ed4f71e 100755\n--- a/gitk\n+++ b/gitk\n@@ -2776,12 +2776,38 @@ proc doprogupdate {} {\n     }\n }\n \n+proc config_init_trace {name} {\n+    global config_variable_changed config_variable_original\n+\n+    upvar #0 $name var\n+    set config_variable_changed($name) 0\n+    set config_variable_original($name) $var\n+}\n+\n+proc config_variable_change_cb {name name2 op} {\n+    global config_variable_changed config_variable_original\n+\n+    upvar #0 $name var\n+    if {$op eq \"write\" &&\n+\t(![info exists config_variable_original($name)] ||\n+\t $config_variable_original($name) ne $var)} {\n+\tset config_variable_changed($name) 1\n+    }\n+}\n+\n proc savestuff {w} {\n-    global viewname viewfiles viewargs viewargscmd viewperm nextviewnum\n-    global use_ttk\n     global stuffsaved\n     global config_file config_file_tmp\n-    global config_variables\n+    global config_variables config_variable_changed\n+    global viewchanged\n+\n+    upvar #0 viewname current_viewname\n+    upvar #0 viewfiles current_viewfiles\n+    upvar #0 viewargs current_viewargs\n+    upvar #0 viewargscmd current_viewargscmd\n+    upvar #0 viewperm current_viewperm\n+    upvar #0 nextviewnum current_nextviewnum\n+    upvar #0 use_ttk current_use_ttk\n \n     if {$stuffsaved} return\n     if {![winfo viewable .]} return\n@@ -2793,16 +2819,24 @@ proc savestuff {w} {\n \tif {$::tcl_platform(platform) eq {windows}} {\n \t    file attributes $config_file_tmp -hidden true\n \t}\n+\tif {[file exists $config_file]} {\n+\t    source $config_file\n+\t}\n \tforeach var_name $config_variables {\n \t    upvar #0 $var_name var\n-\t    puts $f [list set $var_name $var]\n+\t    upvar 0 $var_name old_var\n+\t    if {!$config_variable_changed($var_name) && [info exists old_var]} {\n+\t\tputs $f [list set $var_name $old_var]\n+\t    } else {\n+\t\tputs $f [list set $var_name $var]\n+\t    }\n \t}\n \n \tputs $f \"set geometry(main) [wm geometry .]\"\n \tputs $f \"set geometry(state) [wm state .]\"\n \tputs $f \"set geometry(topwidth) [winfo width .tf]\"\n \tputs $f \"set geometry(topheight) [winfo height .tf]\"\n-\tif {$use_ttk} {\n+\tif {$current_use_ttk} {\n \t    puts $f \"set geometry(pwsash0) \\\"[.tf.histframe.pwclist sashpos 0] 1\\\"\"\n \t    puts $f \"set geometry(pwsash1) \\\"[.tf.histframe.pwclist sashpos 1] 1\\\"\"\n \t} else {\n@@ -2812,11 +2846,34 @@ proc savestuff {w} {\n \tputs $f \"set geometry(botwidth) [winfo width .bleft]\"\n \tputs $f \"set geometry(botheight) [winfo height .bleft]\"\n \n+\tarray set view_save {}\n+\tarray set views {}\n+\tif {![info exists permviews]} { set permviews {} }\n+\tforeach view $permviews {\n+\t    set view_save([lindex $view 0]) 1\n+\t    set views([lindex $view 0]) $view\n+\t}\n \tputs -nonewline $f \"set permviews {\"\n-\tfor {set v 0} {$v < $nextviewnum} {incr v} {\n-\t    if {$viewperm($v)} {\n-\t\tputs $f \"{[list $viewname($v) $viewfiles($v) $viewargs($v) $viewargscmd($v)]}\"\n+\tfor {set v 1} {$v < $current_nextviewnum} {incr v} {\n+\t    if {$viewchanged($v)} {\n+\t\tif {$current_viewperm($v)} {\n+\t\t    set views($current_viewname($v)) [list $current_viewname($v) $current_viewfiles($v) $current_viewargs($v) $current_viewargscmd($v)]\n+\t\t} else {\n+\t\t    set view_save($current_viewname($v)) 0\n+\t\t}\n+\t\tlappend views_modified_names $current_viewname($v)\n+\t    }\n+\t}\n+\t# write old and updated view to their places and append remaining to the end\n+\tforeach view $permviews {\n+\t    set view_name [lindex $view 0]\n+\t    if {$view_save($view_name)} {\n+\t\tputs $f \"{$views($view_name)}\"\n \t    }\n+\t    unset views($view_name)\n+\t}\n+\tforeach view_name [array names views] {\n+\t    puts $f \"{$views($view_name)}\"\n \t}\n \tputs $f \"}\"\n \tclose $f\n@@ -4238,7 +4295,7 @@ proc allviewmenus {n op args} {\n \n proc newviewok {top n {apply 0}} {\n     global nextviewnum newviewperm newviewname newishighlight\n-    global viewname viewfiles viewperm selectedview curview\n+    global viewname viewfiles viewperm viewchanged selectedview curview\n     global viewargs viewargscmd newviewopts viewhlmenu\n \n     if {[catch {\n@@ -4259,6 +4316,7 @@ proc newviewok {top n {apply 0}} {\n \tincr nextviewnum\n \tset viewname($n) $newviewname($n)\n \tset viewperm($n) $newviewopts($n,perm)\n+\tset viewchanged($n) 1\n \tset viewfiles($n) $files\n \tset viewargs($n) $newargs\n \tset viewargscmd($n) $newviewopts($n,cmd)\n@@ -4271,6 +4329,7 @@ proc newviewok {top n {apply 0}} {\n     } else {\n \t# editing an existing view\n \tset viewperm($n) $newviewopts($n,perm)\n+\tset viewchanged($n) 1\n \tif {$newviewname($n) ne $viewname($n)} {\n \t    set viewname($n) $newviewname($n)\n \t    doviewmenu .bar.view 5 [list showview $n] \\\n@@ -4293,7 +4352,7 @@ proc newviewok {top n {apply 0}} {\n }\n \n proc delview {} {\n-    global curview viewperm hlview selectedhlview\n+    global curview viewperm hlview selectedhlview viewchanged\n \n     if {$curview == 0} return\n     if {[info exists hlview] && $hlview == $curview} {\n@@ -4302,6 +4361,7 @@ proc delview {} {\n     }\n     allviewmenus $curview delete\n     set viewperm($curview) 0\n+    set viewchanged($curview) 1\n     showview 0\n }\n \n@@ -12122,6 +12182,10 @@ set config_variables {\n     linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor\n     indexcirclecolor circlecolors linkfgcolor circleoutlinecolor\n }\n+foreach var $config_variables {\n+    config_init_trace $var\n+    trace add variable $var write config_variable_change_cb\n+}\n \n parsefont mainfont $mainfont\n eval font create mainfont [fontflags mainfont]\n@@ -12249,6 +12313,7 @@ set highlight_related [mc \"None\"]\n set highlight_files {}\n set viewfiles(0) {}\n set viewperm(0) 0\n+set viewchanged(0) 0\n set viewargs(0) {}\n set viewargscmd(0) {}\n \n@@ -12307,6 +12372,7 @@ if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {\n     set viewargs(1) $revtreeargs\n     set viewargscmd(1) $revtreeargscmd\n     set viewperm(1) 0\n+    set viewchanged(1) 0\n     set vdatemode(1) 0\n     addviewmenu 1\n     .bar.view entryconf [mca \"Edit view...\"] -state normal\n@@ -12322,6 +12388,7 @@ if {[info exists permviews]} {\n \tset viewargs($n) [lindex $v 2]\n \tset viewargscmd($n) [lindex $v 3]\n \tset viewperm($n) 1\n+\tset viewchanged($n) 0\n \taddviewmenu $n\n     }\n }\n-- \n2.0.1.1697.g73c6810\n"},{"id":"251641","messageId":"1415571602-5858-3-git-send-email-max@max630.net","threadId":"37918","inReplyTo":"1415571602-5858-1-git-send-email-max@max630.net","subject":"[PATCH v4 2/2] gitk: synchronize config write","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-11-09T22:20:02Z","receivedAt":"2014-11-09T22:20:02Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"If several gitk instances are closed simultaneously, safestuff procedure\ncan run at the same time, resulting in a conflict which may cause losing\nof some of the instance's changes, failing the saving operation or even\ncorrupting the configuration file. This can happen, for example, at user\nsession closing, or at group closing of all instances of an application\nwhich is possible in some desktop environments.\n\nTo avoid this, make sure that only one saving operation is in progress.\nIt is guarded by existance of $config_file_tmp file. Both creating the\nfile and moving it to $config_file are atomic operations, so it should\nbe reliable.\n\nReading does not need to be syncronized, because moving is atomic\noperation, and the $config_file always refers to full and correct file.\nBut, if there is a stale $config_file_tmp file, report it at gitk start.\nIf such file is detected at saving, just abort the saving, because this\nis how gitk used to handle errors while saving.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\n gitk | 33 ++++++++++++++++++++++++++++++---\n 1 file changed, 30 insertions(+), 3 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex ed4f71e..692d880 100755\n--- a/gitk\n+++ b/gitk\n@@ -2776,6 +2776,19 @@ proc doprogupdate {} {\n     }\n }\n \n+proc config_check_tmp_exists {tries_left} {\n+    global config_file_tmp\n+\n+    if {[file exists $config_file_tmp]} {\n+\tincr tries_left -1\n+\tif {$tries_left > 0} {\n+\t    after 100 [list config_check_tmp_exists $tries_left]\n+\t} else {\n+\t    error_popup \"Probably there is stale $config_file_tmp file; config saving is going to fail. Check if it is being used by any existing gitk process and remove it otherwise\"\n+\t}\n+    }\n+}\n+\n proc config_init_trace {name} {\n     global config_variable_changed config_variable_original\n \n@@ -2811,11 +2824,16 @@ proc savestuff {w} {\n \n     if {$stuffsaved} return\n     if {![winfo viewable .]} return\n+    set remove_tmp 0\n     catch {\n-\tif {[file exists $config_file_tmp]} {\n-\t    file delete -force $config_file_tmp\n+\tset try_count 0\n+\twhile {[catch {set f [open $config_file_tmp {WRONLY CREAT EXCL}]}]} {\n+\t    if {[incr try_count] > 50} {\n+\t\terror \"Unable to write config file: $config_file_tmp exists\"\n+\t    }\n+\t    after 100\n \t}\n-\tset f [open $config_file_tmp w]\n+\tset remove_tmp 1\n \tif {$::tcl_platform(platform) eq {windows}} {\n \t    file attributes $config_file_tmp -hidden true\n \t}\n@@ -2878,6 +2896,14 @@ proc savestuff {w} {\n \tputs $f \"}\"\n \tclose $f\n \tfile rename -force $config_file_tmp $config_file\n+\tset remove_tmp 0\n+\treturn \"\"\n+    } err\n+    if {$err ne \"\"} {\n+\tputs \"Error saving config: $err\"\n+    }\n+    if {$remove_tmp} {\n+\tfile delete -force $config_file_tmp\n     }\n     set stuffsaved 1\n }\n@@ -12169,6 +12195,7 @@ catch {\n     }\n     source $config_file\n }\n+config_check_tmp_exists 50\n \n set config_variables {\n     mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth\n-- \n2.0.1.1697.g73c6810\n"},{"id":"256814","messageId":"20150301234729.GB24862@iris.ozlabs.ibm.com","threadId":"37918","inReplyTo":"1415571602-5858-2-git-send-email-max@max630.net","subject":"Re: [PATCH v4 1/2] gitk: write only changed configuration variables","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2015-03-01T23:47:30Z","receivedAt":"2015-03-01T23:47:30Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Mon, Nov 10, 2014 at 12:20:01AM +0200, Max Kirillov wrote:\n> When gitk contains some changed parameter, and there is existing\n> instance of gitk where the parameter is still old, it is reverted to\n> that old value when the instance exits.\n> \n> Instead, store a parameter in config only it is has been modified in the\n> exiting instance. Otherwise, preserve the value which currently is in\n> file.  This allows editing the configuration when several instances are\n> running, and don't get rollback of the modification if some other\n> instance where the configuration was not edited is closed last.\n> \n> For scalar variables, use trace(3tcl) to detect their change. Since `trace` can\n> send bogus events, doublecheck if the value has really been changed, but once\n> it is marked as changed, do not reset it back to unchanged ever, because if\n> user has restored the original value, it's the decision which should be stored\n> as well as modified value.\n> \n> Treat view list especially: instead of rewriting the whole list, merge\n> individual views. Place old and updated views at their older placed, add\n> new ones to the end of list. Collect modified view explicitly, in newviewok{}\n> and delview{}.\n> \n> Do not merge geometry values. They are almost always changing because\n> user moves and resises windows, and there is no way to find which one of\n> the geometries is most desired. Just overwrite them unconditionally,\n> like earlier.\n> \n> Signed-off-by: Max Kirillov <max@max630.net>\n\nLooks pretty nice; I just have one comment:\n\n> +\t\tlappend views_modified_names $current_viewname($v)\n\nThis view_modified_names variable doesn't seem to be used anywhere.\nIf you don't mind me taking out this line, I'll do that and apply the\npatch.\n\nRegards,\nPaul.\n"},{"id":"256813","messageId":"20150302001050.GC24862@iris.ozlabs.ibm.com","threadId":"37918","inReplyTo":"1415571602-5858-3-git-send-email-max@max630.net","subject":"Re: [PATCH v4 2/2] gitk: synchronize config write","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2015-03-02T00:10:51Z","receivedAt":"2015-03-02T00:10:51Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Mon, Nov 10, 2014 at 12:20:02AM +0200, Max Kirillov wrote:\n> If several gitk instances are closed simultaneously, safestuff procedure\n> can run at the same time, resulting in a conflict which may cause losing\n> of some of the instance's changes, failing the saving operation or even\n> corrupting the configuration file. This can happen, for example, at user\n> session closing, or at group closing of all instances of an application\n> which is possible in some desktop environments.\n> \n> To avoid this, make sure that only one saving operation is in progress.\n> It is guarded by existance of $config_file_tmp file. Both creating the\n> file and moving it to $config_file are atomic operations, so it should\n> be reliable.\n> \n> Reading does not need to be syncronized, because moving is atomic\n> operation, and the $config_file always refers to full and correct file.\n> But, if there is a stale $config_file_tmp file, report it at gitk start.\n> If such file is detected at saving, just abort the saving, because this\n> is how gitk used to handle errors while saving.\n> \n> Signed-off-by: Max Kirillov <max@max630.net>\n\nThe idea looks good; I have a couple of comments on the patch.  First,\n50 tries over 5 seconds seems a bit excessive to me, wouldn't (say) 20\ntries be enough?  Is the 50 the result of some analysis?\n\n> +\t    error_popup \"Probably there is stale $config_file_tmp file; config saving is going to fail. Check if it is being used by any existing gitk process and remove it otherwise\"\n\nI would word this as \"There appears to be a stale $config_file_tmp\nfile, which will prevent gitk from saving its configuration on exit.\nPlease remove it if it is not being used by any existing gitk\nprocess.\"\n\n> @@ -2811,11 +2824,16 @@ proc savestuff {w} {\n>  \n>      if {$stuffsaved} return\n>      if {![winfo viewable .]} return\n> +    set remove_tmp 0\n>      catch {\n> -\tif {[file exists $config_file_tmp]} {\n> -\t    file delete -force $config_file_tmp\n> +\tset try_count 0\n> +\twhile {[catch {set f [open $config_file_tmp {WRONLY CREAT EXCL}]}]} {\n> +\t    if {[incr try_count] > 50} {\n> +\t\terror \"Unable to write config file: $config_file_tmp exists\"\n> +\t    }\n> +\t    after 100\n>  \t}\n> -\tset f [open $config_file_tmp w]\n> +\tset remove_tmp 1\n>  \tif {$::tcl_platform(platform) eq {windows}} {\n>  \t    file attributes $config_file_tmp -hidden true\n>  \t}\n> @@ -2878,6 +2896,14 @@ proc savestuff {w} {\n>  \tputs $f \"}\"\n>  \tclose $f\n>  \tfile rename -force $config_file_tmp $config_file\n> +\tset remove_tmp 0\n> +\treturn \"\"\n> +    } err\n> +    if {$err ne \"\"} {\n> +\tputs \"Error saving config: $err\"\n\nI would suggest checking the return from the catch statement, like\nthis:\n\n\tif {[catch {\n\t    ...\n\t    file rename -force $config_file_tmp $config_file\n\t} err]} {\n\t    puts \"Error saving config: $err\"\n\t    if {$remove_tmp} {\n\t\tfile delete -force $config_file_tmp\n\t    }\n\t}\n\nrather than doing a return inside the catch.\n\nPaul.\n"},{"id":"256866","messageId":"20150302203406.GA7622@wheezy.local","threadId":"37918","inReplyTo":"20150301234729.GB24862@iris.ozlabs.ibm.com","subject":"Re: [PATCH v4 1/2] gitk: write only changed configuration variables","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-03-02T20:34:06Z","receivedAt":"2015-03-02T20:34:06Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Mon, Mar 02, 2015 at 10:47:30AM +1100, Paul Mackerras wrote:\n> On Mon, Nov 10, 2014 at 12:20:01AM +0200, Max Kirillov wrote:\n>> +\t\tlappend views_modified_names $current_viewname($v)\n> \n> This view_modified_names variable doesn't seem to be used anywhere.\n> If you don't mind me taking out this line, I'll do that and apply the\n> patch.\n\nYes, sure. Thank you. Must be some editing leftover. I could\nnot recover what was my intention, even if something\nmeaningful was ever committed it is rebased and gc-ed\nby now.\n\n-- \nMax\n"},{"id":"256867","messageId":"20150302204354.GB7622@wheezy.local","threadId":"37918","inReplyTo":"20150302001050.GC24862@iris.ozlabs.ibm.com","subject":"Re: [PATCH v4 2/2] gitk: synchronize config write","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-03-02T20:43:54Z","receivedAt":"2015-03-02T20:43:54Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Mon, Mar 02, 2015 at 11:10:51AM +1100, Paul Mackerras wrote:\n> The idea looks good; I have a couple of comments on the patch.  First,\n> 50 tries over 5 seconds seems a bit excessive to me, wouldn't (say) 20\n> tries be enough?  Is the 50 the result of some analysis?\n\n5 seconds was just my personal feeling where \"too long\" is\nstarting.\n\nI have made some quick experiment: I opened ~10 instances\nand closing them with group close in Windows 7. With 1\nsecond wait it often can hit the error while closing several\ninstances at once. With 20 seemed to work reliably. Will\nlook a bit more though, maybe also handle the failing case\nmore nice, to avoid having leftover.\n\nThe interval of 100 milliseconds was also voluntary. Maybe\nneed to measure how long the saving actually takes.\n\n> \n>> +\t    error_popup \"Probably there is stale $config_file_tmp file; config saving is going to fail. Check if it is being used by any existing gitk process and remove it otherwise\"\n> \n> I would word this as \"There appears to be a stale $config_file_tmp\n> file, which will prevent gitk from saving its configuration on exit.\n> Please remove it if it is not being used by any existing gitk\n> process.\"\n\nok, will change it\n\n> > @@ -2811,11 +2824,16 @@ proc savestuff {w} {\n> >  \n> >      if {$stuffsaved} return\n> >      if {![winfo viewable .]} return\n> > +    set remove_tmp 0\n> >      catch {\n> > -\tif {[file exists $config_file_tmp]} {\n> > -\t    file delete -force $config_file_tmp\n> > +\tset try_count 0\n> > +\twhile {[catch {set f [open $config_file_tmp {WRONLY CREAT EXCL}]}]} {\n> > +\t    if {[incr try_count] > 50} {\n> > +\t\terror \"Unable to write config file: $config_file_tmp exists\"\n> > +\t    }\n> > +\t    after 100\n> >  \t}\n> > -\tset f [open $config_file_tmp w]\n> > +\tset remove_tmp 1\n> >  \tif {$::tcl_platform(platform) eq {windows}} {\n> >  \t    file attributes $config_file_tmp -hidden true\n> >  \t}\n> > @@ -2878,6 +2896,14 @@ proc savestuff {w} {\n> >  \tputs $f \"}\"\n> >  \tclose $f\n> >  \tfile rename -force $config_file_tmp $config_file\n> > +\tset remove_tmp 0\n> > +\treturn \"\"\n> > +    } err\n> > +    if {$err ne \"\"} {\n> > +\tputs \"Error saving config: $err\"\n> \n> I would suggest checking the return from the catch statement, like\n> this:\n> ...\n> rather than doing a return inside the catch.\n\nYes, I can make proper error handling. Then I think it would better be a separated patch.\n\n-- \nMax\n"}]}