{"thread":{"id":"37566","subject":"[PATCH v2 0/3] gitk: save only changed configuration on exit","startedAt":"2014-09-14T20:35:56Z","lastAt":"2014-10-30T21:43:57Z","messageCount":7,"participants":["Max Kirillov","Paul Mackerras"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"249407","messageId":"1410726959-20353-1-git-send-email-max@max630.net","threadId":"37566","inReplyTo":null,"subject":"[PATCH v2 0/3] gitk: save only changed configuration on exit","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-09-14T20:35:56Z","receivedAt":"2014-09-14T20:35:56Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Changes since v1:\n\n* Add value check to config_variable_change_cb\n* Squash 2/3 and 3/3 and the value check into one commit. There is no really\n  reasons to divide them except following the real coding history\n* Describle the previous undesirable behavior in commit message\n* Synchronize writing of config file\n\nMax Kirillov (3):\n  gitk refactor: remove boilerplate for configuration variables\n  gitk: write only changed configuration variables\n  gitk: synchronize config write\n\n gitk | 215 +++++++++++++++++++++++++++++++++++++++++++------------------------\n 1 file changed, 139 insertions(+), 76 deletions(-)\n\n-- \n2.0.1.1697.g73c6810\n"},{"id":"249409","messageId":"1410726959-20353-2-git-send-email-max@max630.net","threadId":"37566","inReplyTo":"1410726959-20353-1-git-send-email-max@max630.net","subject":"[PATCH v2 1/3] gitk refactor: remove boilerplate for configuration variables","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-09-14T20:35:57Z","receivedAt":"2014-09-14T20:35:57Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Signed-off-by: Max Kirillov <max@max630.net>\n---\n gitk | 88 ++++++++++++++++----------------------------------------------------\n 1 file changed, 20 insertions(+), 68 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex c8df35d..bc57c11 100755\n--- a/gitk\n+++ b/gitk\n@@ -2772,23 +2772,11 @@ proc doprogupdate {} {\n }\n \n proc savestuff {w} {\n-    global canv canv2 canv3 mainfont textfont uifont tabstop\n-    global stuffsaved findmergefiles maxgraphpct\n-    global maxwidth showneartags showlocalchanges\n     global viewname viewfiles viewargs viewargscmd viewperm nextviewnum\n-    global cmitmode wrapcomment datetimeformat limitdiffs\n-    global colors uicolor bgcolor fgcolor diffcolors diffcontext selectbgcolor\n-    global uifgcolor uifgdisabledcolor\n-    global headbgcolor headfgcolor headoutlinecolor remotebgcolor\n-    global tagbgcolor tagfgcolor tagoutlinecolor\n-    global reflinecolor filesepbgcolor filesepfgcolor\n-    global mergecolors foundbgcolor currentsearchhitbgcolor\n-    global linehoverbgcolor linehoverfgcolor linehoveroutlinecolor circlecolors\n-    global mainheadcirclecolor workingfilescirclecolor indexcirclecolor\n-    global linkfgcolor circleoutlinecolor\n-    global autoselect autosellen extdifftool perfile_attrs markbgcolor use_ttk\n-    global hideremotes want_ttk maxrefs visiblerefs\n+    global use_ttk\n+    global stuffsaved\n     global config_file config_file_tmp\n+    global config_variables\n \n     if {$stuffsaved} return\n     if {![winfo viewable .]} return\n@@ -2800,59 +2788,10 @@ proc savestuff {w} {\n \tif {$::tcl_platform(platform) eq {windows}} {\n \t    file attributes $config_file_tmp -hidden true\n \t}\n-\tputs $f [list set mainfont $mainfont]\n-\tputs $f [list set textfont $textfont]\n-\tputs $f [list set uifont $uifont]\n-\tputs $f [list set tabstop $tabstop]\n-\tputs $f [list set findmergefiles $findmergefiles]\n-\tputs $f [list set maxgraphpct $maxgraphpct]\n-\tputs $f [list set maxwidth $maxwidth]\n-\tputs $f [list set cmitmode $cmitmode]\n-\tputs $f [list set wrapcomment $wrapcomment]\n-\tputs $f [list set autoselect $autoselect]\n-\tputs $f [list set autosellen $autosellen]\n-\tputs $f [list set showneartags $showneartags]\n-\tputs $f [list set maxrefs $maxrefs]\n-\tputs $f [list set visiblerefs $visiblerefs]\n-\tputs $f [list set hideremotes $hideremotes]\n-\tputs $f [list set showlocalchanges $showlocalchanges]\n-\tputs $f [list set datetimeformat $datetimeformat]\n-\tputs $f [list set limitdiffs $limitdiffs]\n-\tputs $f [list set uicolor $uicolor]\n-\tputs $f [list set want_ttk $want_ttk]\n-\tputs $f [list set bgcolor $bgcolor]\n-\tputs $f [list set fgcolor $fgcolor]\n-\tputs $f [list set uifgcolor $uifgcolor]\n-\tputs $f [list set uifgdisabledcolor $uifgdisabledcolor]\n-\tputs $f [list set colors $colors]\n-\tputs $f [list set diffcolors $diffcolors]\n-\tputs $f [list set mergecolors $mergecolors]\n-\tputs $f [list set markbgcolor $markbgcolor]\n-\tputs $f [list set diffcontext $diffcontext]\n-\tputs $f [list set selectbgcolor $selectbgcolor]\n-\tputs $f [list set foundbgcolor $foundbgcolor]\n-\tputs $f [list set currentsearchhitbgcolor $currentsearchhitbgcolor]\n-\tputs $f [list set extdifftool $extdifftool]\n-\tputs $f [list set perfile_attrs $perfile_attrs]\n-\tputs $f [list set headbgcolor $headbgcolor]\n-\tputs $f [list set headfgcolor $headfgcolor]\n-\tputs $f [list set headoutlinecolor $headoutlinecolor]\n-\tputs $f [list set remotebgcolor $remotebgcolor]\n-\tputs $f [list set tagbgcolor $tagbgcolor]\n-\tputs $f [list set tagfgcolor $tagfgcolor]\n-\tputs $f [list set tagoutlinecolor $tagoutlinecolor]\n-\tputs $f [list set reflinecolor $reflinecolor]\n-\tputs $f [list set filesepbgcolor $filesepbgcolor]\n-\tputs $f [list set filesepfgcolor $filesepfgcolor]\n-\tputs $f [list set linehoverbgcolor $linehoverbgcolor]\n-\tputs $f [list set linehoverfgcolor $linehoverfgcolor]\n-\tputs $f [list set linehoveroutlinecolor $linehoveroutlinecolor]\n-\tputs $f [list set mainheadcirclecolor $mainheadcirclecolor]\n-\tputs $f [list set workingfilescirclecolor $workingfilescirclecolor]\n-\tputs $f [list set indexcirclecolor $indexcirclecolor]\n-\tputs $f [list set circlecolors $circlecolors]\n-\tputs $f [list set linkfgcolor $linkfgcolor]\n-\tputs $f [list set circleoutlinecolor $circleoutlinecolor]\n+\tforeach var_name $config_variables {\n+\t    upvar #0 $var_name var\n+\t    puts $f [list set $var_name $var]\n+\t}\n \n \tputs $f \"set geometry(main) [wm geometry .]\"\n \tputs $f \"set geometry(state) [wm state .]\"\n@@ -12157,6 +12096,19 @@ catch {\n     source $config_file\n }\n \n+set config_variables {\n+    mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth\n+    cmitmode wrapcomment autoselect autosellen showneartags maxrefs visiblerefs\n+    hideremotes showlocalchanges datetimeformat limitdiffs uicolor want_ttk\n+    bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors\n+    markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor\n+    extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor\n+    remotebgcolor tagbgcolor tagfgcolor tagoutlinecolor reflinecolor\n+    filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor\n+    linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor\n+    indexcirclecolor circlecolors linkfgcolor circleoutlinecolor\n+}\n+\n parsefont mainfont $mainfont\n eval font create mainfont [fontflags mainfont]\n eval font create mainfontbold [fontflags mainfont 1]\n-- \n2.0.1.1697.g73c6810\n"},{"id":"249410","messageId":"1410726959-20353-3-git-send-email-max@max630.net","threadId":"37566","inReplyTo":"1410726959-20353-1-git-send-email-max@max630.net","subject":"[PATCH v2 2/3] gitk: write only changed configuration variables","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-09-14T20:35:58Z","receivedAt":"2014-09-14T20:35:58Z","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\nSince trace(3tcl) can send bogus events, doublecheck if the value has\nreally been changed, but once it is marked as changed, do not reset it\nback to unchanged ever, because if user has restored the original value,\nit's the decision which should be stored as 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.\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 | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------\n 1 file changed, 92 insertions(+), 8 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex bc57c11..e76445b 100755\n--- a/gitk\n+++ b/gitk\n@@ -2771,12 +2771,51 @@ proc doprogupdate {} {\n     }\n }\n \n+proc config_resolve_variable {name name2} {\n+    set var_name \"\"\n+    set key \"\"\n+    if {$name2 eq \"\"} {\n+\tset key $name\n+\tset var_name $name\n+    } else {\n+\tset key \"$name,$name2\"\n+\tset var_name \"${name}($name2)\"\n+    }\n+    upvar #0 $var_name var\n+    return [list $key $var]\n+}\n+\n+proc config_init_trace {name name2} {\n+    global config_variable_changed config_variable_original\n+\n+    set var_info [config_resolve_variable $name $name2]\n+    set config_variable_changed([lindex $var_info 0]) 0\n+    set config_variable_original([lindex $var_info 0]) [lindex $var_info 1]\n+}\n+\n+proc config_variable_change_cb {name name2 op} {\n+    global config_variable_changed config_variable_original\n+\n+    set var_info [config_resolve_variable $name $name2]\n+    if {$op eq \"write\" &&\n+\t([llength [array names config_variable_original -exact [lindex $var_info 0]]] == 0 ||\n+\t $config_variable_original([lindex $var_info 0]) ne [lindex $var_info 1])} {\n+\tset config_variable_changed([lindex $var_info 0]) 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+\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@@ -2788,16 +2827,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@@ -2807,12 +2854,39 @@ 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 {$config_variable_changed(viewname,$v) ||\n+\t\t$config_variable_changed(viewfiles,$v) ||\n+\t\t$config_variable_changed(viewargs,$v) ||\n+\t\t$config_variable_changed(viewargscmd,$v) ||\n+\t\t$config_variable_changed(viewperm,$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 \tfile rename -force $config_file_tmp $config_file\n@@ -12108,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@@ -12311,6 +12389,12 @@ if {[info exists permviews]} {\n \taddviewmenu $n\n     }\n }\n+foreach var {viewname viewfiles viewargs viewargscmd viewperm} {\n+    for {set v 1} {$v < $nextviewnum} {incr v} {\n+\tconfig_init_trace $var $v\n+    }\n+    trace add variable $var write config_variable_change_cb\n+}\n \n if {[tk windowingsystem] eq \"win32\"} {\n     focus -force .\n-- \n2.0.1.1697.g73c6810\n"},{"id":"249408","messageId":"1410726959-20353-4-git-send-email-max@max630.net","threadId":"37566","inReplyTo":"1410726959-20353-1-git-send-email-max@max630.net","subject":"[PATCH v2 3/3] gitk: synchronize config write","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-09-14T20:35:59Z","receivedAt":"2014-09-14T20:35:59Z","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 existence 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 e76445b..c65103e 100755\n--- a/gitk\n+++ b/gitk\n@@ -2771,6 +2771,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_resolve_variable {name name2} {\n     set var_name \"\"\n     set key \"\"\n@@ -2819,11 +2832,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@@ -2890,6 +2908,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":"251210","messageId":"20141030094757.GD16472@iris.ozlabs.ibm.com","threadId":"37566","inReplyTo":"1410726959-20353-2-git-send-email-max@max630.net","subject":"Re: [PATCH v2 1/3] gitk refactor: remove boilerplate for configuration variables","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2014-10-30T09:47:57Z","receivedAt":"2014-10-30T09:47:57Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Sun, Sep 14, 2014 at 11:35:57PM +0300, Max Kirillov wrote:\n> Signed-off-by: Max Kirillov <max@max630.net>\n\nThis is a nice simplification, applied, thanks.\n\nPaul.\n"},{"id":"251211","messageId":"20141030095513.GE16472@iris.ozlabs.ibm.com","threadId":"37566","inReplyTo":"1410726959-20353-3-git-send-email-max@max630.net","subject":"Re: [PATCH v2 2/3] gitk: write only changed configuration variables","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2014-10-30T09:55:13Z","receivedAt":"2014-10-30T09:55:13Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Sun, Sep 14, 2014 at 11:35:58PM +0300, 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> Since trace(3tcl) can send bogus events, doublecheck if the value has\n> really been changed, but once it is marked as changed, do not reset it\n> back to unchanged ever, because if user has restored the original value,\n> it's the decision which should be stored 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.\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\nI like the idea here but the implementation seems a bit more\ncomplicated than it needs to be.  It seems to me that we need the\ntrace only for the non-array configuration variables; the array case\nis only for the view definitions, and I think we could just set the\nchanged flag for a view explicitly in [newviewok].  That would\nsimplify things quite a bit.\n\nI'm also not convinced we need all the uses of upvar.  Why do we need\nto use upvar to rename viewname, viewfiles etc. to current_viewname,\netc.?  If you're concerned about what might possibly be in the .gitk\nwhen you source it, perhaps doing the source inside a namespace would\nbe a cleaner approach?\n\nPaul.\n"},{"id":"251246","messageId":"20141030214357.GA6484@wheezy.local","threadId":"37566","inReplyTo":"20141030095513.GE16472@iris.ozlabs.ibm.com","subject":"Re: [PATCH v2 2/3] gitk: write only changed configuration variables","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-10-30T21:43:57Z","receivedAt":"2014-10-30T21:43:57Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Hi.\n\nOn Thu, Oct 30, 2014 at 08:55:13PM +1100, Paul Mackerras wrote:\n> It seems to me that we need the trace only for the\n> non-array configuration variables; the array case is only\n> for the view definitions, and I think we could just set\n> the changed flag for a view explicitly in [newviewok].\n> That would simplify things quite a bit.\n\nI liked the idea to exploit that tcl can watch array element\nuniformly with scalar variables. But I agree that the result\nis a bit complicated. I will try to use the explicit flag and\nsee how it's going to look.\n\n> On Sun, Sep 14, 2014 at 11:35:58PM +0300, Max Kirillov wrote:\n> I'm also not convinced we need all the uses of upvar.  Why do we need\n> to use upvar to rename viewname, viewfiles etc. to current_viewname,\n> etc.?  If you're concerned about what might possibly be in the .gitk\n> when you source it, perhaps doing the source inside a namespace would\n> be a cleaner approach?\n\nI have tried namespaces originally but failed with them.\nApparently when \"set v ..\" runs in namespace and global v\nexists, it modifies the global v rather than creates a new\nvariabe in a namespace. I don't remember all details now but\nI could not find how to make it with namespaces. I should\nsay I had not known anything about tcl namespaces before\nI started doing this but, so maybe I missed something.\n\n-- \nMax\n\nPS: the script which shows the namespace behavior:\n\n> set a 1\n> namespace eval ns1 {\n>  set a 2\n> }\n> puts \"a=$a\"\n> puts \"ns1::a=$ns1::a\"\n\nOutput is:\n> a=2\n> can't read \"ns1::a\": no such variable\n>    while executing\n> \"puts \"ns1::a=$ns1::a\"\"\n"}]}