{"thread":{"id":"52996","subject":"[PATCH v1 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","startedAt":"2020-03-15T01:47:32Z","lastAt":"2020-03-19T16:05:58Z","messageCount":12,"participants":["Pratyush Yadav","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"393242","messageId":"20200314224159.14174-1-me@yadavpratyush.com","threadId":"52996","inReplyTo":null,"subject":"[PATCH v1 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-03-14T22:41:57Z","receivedAt":"2020-03-15T01:47:32Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi,\n\nSome MacOS distributions ship with Tcl 8.5. This means we can't use\nTclOO. So, use our homegrown class.tcl instead.\n\nWhile here, fix a potential variable name collision by creating a\nseparate namespace for a chord's script evaluation.\n\nJonathan,\n\nCan you please test the patches the same way you tested your original\nseries just to be sure we don't break anything? A review would also be\nnice.\n\nPratyush Yadav (2):\n  git-gui: reduce Tcl version requirement from 8.6 to 8.5\n  git-gui: create a new namespace for chord script evaluation\n\n git-gui.sh    |  4 ++--\n lib/chord.tcl | 56 +++++++++++++++++++++++++--------------------------\n lib/index.tcl | 10 +++++----\n 3 files changed, 35 insertions(+), 35 deletions(-)\n\n--\n2.26.0.rc1.11.g30e9940356\n\n"},{"id":"393244","messageId":"20200314224159.14174-3-me@yadavpratyush.com","threadId":"52996","inReplyTo":"20200314224159.14174-1-me@yadavpratyush.com","subject":"[PATCH v1 2/2] git-gui: create a new namespace for chord script evaluation","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-03-14T22:41:59Z","receivedAt":"2020-03-15T01:48:41Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Evaluating the script in the same namespace as the chord itself creates\npotential for variable name collision. And in that case the script would\nunknowingly use the chord's variables.\n\nFor example, say the script has a variable called 'is_completed', which\nalso exists in the chord's namespace. The script then calls 'eval' and\nsets 'is_completed' to 1 thinking it is setting its own variable,\ncompletely unaware of how the chord works behind the scenes. This leads\nto the chord never actually executing because it sees 'is_completed' as\ntrue and thinks it has already completed.\n\nAvoid the potential collision by creating a separate namespace for the\nscript that is a child of the chord's namespace.\n\nSigned-off-by: Pratyush Yadav <me@yadavpratyush.com>\n---\n lib/chord.tcl | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/lib/chord.tcl b/lib/chord.tcl\nindex 7de7cba..e21e7d3 100644\n--- a/lib/chord.tcl\n+++ b/lib/chord.tcl\n@@ -64,6 +64,7 @@ class SimpleChord {\n \tfield notes\n \tfield body\n \tfield is_completed\n+\tfield eval_ns\n \n \t# Constructor:\n \t#   set chord [SimpleChord::new {body}]\n@@ -74,6 +75,7 @@ class SimpleChord {\n \t\tset notes [list]\n \t\tset body $i_body\n \t\tset is_completed 0\n+\t\tset eval_ns \"[namespace qualifiers $this]::eval\"\n \t\treturn $this\n \t}\n \n@@ -83,7 +85,7 @@ class SimpleChord {\n \t#     the chord body will be evaluated. This can be used to set variable\n \t#     values for the chord body to use.\n \tmethod eval {script} {\n-\t\tnamespace eval [namespace qualifiers $this] $script\n+\t\tnamespace eval $eval_ns $script\n \t}\n \n \t# Method:\n@@ -111,7 +113,7 @@ class SimpleChord {\n \n \t\t\tset is_completed 1\n \n-\t\t\tnamespace eval [namespace qualifiers $this] $body\n+\t\t\tnamespace eval $eval_ns $body\n \t\t\tdelete_this\n \t\t}\n \t}\n-- \n2.26.0.rc1.11.g30e9940356\n\n"},{"id":"393245","messageId":"20200314224159.14174-2-me@yadavpratyush.com","threadId":"52996","inReplyTo":"20200314224159.14174-1-me@yadavpratyush.com","subject":"[PATCH v1 1/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-03-14T22:41:58Z","receivedAt":"2020-03-15T01:54:06Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On some MacOS distributions like High Sierra, Tcl 8.5 is shipped by\ndefault. This makes git-gui error out at startup because of the version\nmismatch.\n\nThe only part that requires Tcl 8.6 is SimpleChord, which depends on\nTclOO. So, don't use it and use our homegrown class.tcl instead.\n\nThis means some slight syntax changes. Since class.tcl doesn't have an\n\"unknown\" method like TclOO does, we can't just call '$note', but have\nto use '$note activate' instead. The constructor now needs a proper\nnamespace qualifier. Update the documentation to reflect the new syntax.\n\nAs of now, the only part of git-gui that needs Tcl 8.5 is a call to\n'apply' in lib/index.tcl::lambda. Keep using it until someone shows up\nshouting that their OS ships with 8.4 only. Then we would have to look\ninto implementing it in pure Tcl.\n\nSigned-off-by: Pratyush Yadav <me@yadavpratyush.com>\n---\n git-gui.sh    |  4 ++--\n lib/chord.tcl | 54 ++++++++++++++++++++++++---------------------------\n lib/index.tcl | 10 ++++++----\n 3 files changed, 33 insertions(+), 35 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex f41ed2e..027e093 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -30,8 +30,8 @@ along with this program; if not, see <http://www.gnu.org/licenses/>.}]\n ##\n ## Tcl/Tk sanity check\n\n-if {[catch {package require Tcl 8.6} err]\n- || [catch {package require Tk  8.6} err]\n+if {[catch {package require Tcl 8.5} err]\n+ || [catch {package require Tk  8.5} err]\n } {\n \tcatch {wm withdraw .}\n \ttk_messageBox \\\ndiff --git a/lib/chord.tcl b/lib/chord.tcl\nindex 275a6cd..7de7cba 100644\n--- a/lib/chord.tcl\n+++ b/lib/chord.tcl\n@@ -27,7 +27,7 @@\n #   # Turn off the UI while running a couple of async operations.\n #   lock_ui\n #\n-#   set chord [SimpleChord new {\n+#   set chord [SimpleChord::new {\n #     unlock_ui\n #     # Note: $notice here is not referenced in the calling scope\n #     if {$notice} { info_popup $notice }\n@@ -37,9 +37,9 @@\n #   # all operations have been initiated.\n #   set common_note [$chord add_note]\n #\n-#   # Pass notes as 'after' callbacks to other operations\n-#   async_operation $args [$chord add_note]\n-#   other_async_operation $args [$chord add_note]\n+#   # Activate notes in 'after' callbacks to other operations\n+#   set newnote [$chord add_note]\n+#   async_operation $args [list $newnote activate]\n #\n #   # Communicate with the chord body\n #   if {$condition} {\n@@ -48,7 +48,7 @@\n #   }\n #\n #   # Activate the common note, making the chord eligible to complete\n-#   $common_note\n+#   $common_note activate\n #\n # At this point, the chord will complete at some unknown point in the future.\n # The common note might have been the first note activated, or the async\n@@ -60,18 +60,21 @@\n #   Represents a procedure that conceptually has multiple entrypoints that must\n #   all be called before the procedure executes. Each entrypoint is called a\n #   \"note\". The chord is only \"completed\" when all the notes are \"activated\".\n-oo::class create SimpleChord {\n-\tvariable notes body is_completed\n+class SimpleChord {\n+\tfield notes\n+\tfield body\n+\tfield is_completed\n\n \t# Constructor:\n-\t#   set chord [SimpleChord new {body}]\n+\t#   set chord [SimpleChord::new {body}]\n \t#     Creates a new chord object with the specified body script. The\n \t#     body script is evaluated at most once, when a note is activated\n \t#     and the chord has no other non-activated notes.\n-\tconstructor {body} {\n+\tconstructor new {i_body} {\n \t\tset notes [list]\n-\t\tmy eval [list set body $body]\n+\t\tset body $i_body\n \t\tset is_completed 0\n+\t\treturn $this\n \t}\n\n \t# Method:\n@@ -80,7 +83,7 @@ oo::class create SimpleChord {\n \t#     the chord body will be evaluated. This can be used to set variable\n \t#     values for the chord body to use.\n \tmethod eval {script} {\n-\t\tnamespace eval [self] $script\n+\t\tnamespace eval [namespace qualifiers $this] $script\n \t}\n\n \t# Method:\n@@ -92,7 +95,7 @@ oo::class create SimpleChord {\n \tmethod add_note {} {\n \t\tif {$is_completed} { error \"Cannot add a note to a completed chord\" }\n\n-\t\tset note [ChordNote new [self]]\n+\t\tset note [ChordNote::new $this]\n\n \t\tlappend notes $note\n\n@@ -108,8 +111,8 @@ oo::class create SimpleChord {\n\n \t\t\tset is_completed 1\n\n-\t\t\tnamespace eval [self] $body\n-\t\t\tnamespace delete [self]\n+\t\t\tnamespace eval [namespace qualifiers $this] $body\n+\t\t\tdelete_this\n \t\t}\n \t}\n }\n@@ -119,15 +122,17 @@ oo::class create SimpleChord {\n #   final note of the chord is activated (this can be any note in the chord,\n #   with all other notes already previously activated in any order), the chord's\n #   body is evaluated.\n-oo::class create ChordNote {\n-\tvariable chord is_activated\n+class ChordNote {\n+\tfield chord\n+\tfield is_activated\n\n \t# Constructor:\n \t#   Instances of ChordNote are created internally by calling add_note on\n \t#   SimpleChord objects.\n-\tconstructor {chord} {\n-\t\tmy eval set chord $chord\n+\tconstructor new {c} {\n+\t\tset chord $c\n \t\tset is_activated 0\n+\t\treturn $this\n \t}\n\n \t# Method:\n@@ -138,20 +143,11 @@ oo::class create ChordNote {\n \t}\n\n \t# Method:\n-\t#   $note\n+\t#   $note activate\n \t#     Activates the note, if it has not already been activated, and\n \t#     completes the chord if there are no other notes awaiting\n \t#     activation. Subsequent calls will have no further effect.\n-\t#\n-\t# NB: In TclOO, if an object is invoked like a method without supplying\n-\t#     any method name, then this internal method `unknown` is what\n-\t#     actually runs (with no parameters). It is used in the ChordNote\n-\t#     class for the purpose of allowing the note object to be called as\n-\t#     a function (see example above). (The `unknown` method can also be\n-\t#     used to support dynamic dispatch, but must take parameters to\n-\t#     identify the \"unknown\" method to be invoked. In this form, this\n-\t#     proc serves only to make instances behave directly like methods.)\n-\tmethod unknown {} {\n+\tmethod activate {} {\n \t\tif {!$is_activated} {\n \t\t\tset is_activated 1\n \t\t\t$chord notify_note_activation\ndiff --git a/lib/index.tcl b/lib/index.tcl\nindex 1254145..ebcd6e4 100644\n--- a/lib/index.tcl\n+++ b/lib/index.tcl\n@@ -436,7 +436,7 @@ proc revert_helper {txt paths} {\n \t#\n \t# The asynchronous operations are each indicated below by a comment\n \t# before the code block that starts the async operation.\n-\tset after_chord [SimpleChord new {\n+\tset after_chord [SimpleChord::new {\n \t\tif {[string trim $err] != \"\"} {\n \t\t\trescan_on_error $err\n \t\t} else {\n@@ -521,11 +521,12 @@ proc revert_helper {txt paths} {\n \t\t\t[mc \"Revert Changes\"] \\\n \t\t\t]\n\n+\t\tset note [$after_chord add_note]\n \t\tif {$reply == 1} {\n \t\t\tcheckout_index \\\n \t\t\t\t$txt \\\n \t\t\t\t$path_list \\\n-\t\t\t\t[$after_chord add_note] \\\n+\t\t\t\t[list $note activate] \\\n \t\t\t\t$capture_error\n \t\t}\n \t}\n@@ -564,17 +565,18 @@ proc revert_helper {txt paths} {\n \t\t\t[mc \"Delete Files\"] \\\n \t\t\t]\n\n+\t\tset note [$after_chord add_note]\n \t\tif {$reply == 1} {\n \t\t\t$after_chord eval { set should_reshow_diff 1 }\n\n-\t\t\tdelete_files $untracked_list [$after_chord add_note]\n+\t\t\tdelete_files $untracked_list [list $note activate]\n \t\t}\n \t}\n\n \t# Activate the common note. If no other notes were created, this\n \t# completes the chord. If other notes were created, then this common\n \t# note prevents a race condition where the chord might complete early.\n-\t$after_common_note\n+\t$after_common_note activate\n }\n\n # Delete all of the specified files, performing deletion in batches to allow the\n--\n2.26.0.rc1.11.g30e9940356\n\n"},{"id":"393281","messageId":"CAPig+cRXD_bjUL6=daEAx7VnAy_nw9bao6rLK9xwTCYJSk48Qw@mail.gmail.com","threadId":"52996","inReplyTo":"20200314224159.14174-1-me@yadavpratyush.com","subject":"Re: [PATCH v1 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-15T18:54:07Z","receivedAt":"2020-03-15T18:54:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Mar 14, 2020 at 9:47 PM Pratyush Yadav <me@yadavpratyush.com> wrote:\n> Some MacOS distributions ship with Tcl 8.5. This means we can't use\n> TclOO. So, use our homegrown class.tcl instead.\n\nIt should be mentioned that this patch series fixes a regression in\nGit v2.25 in which git-gui could not even be launched on Mac OS. The\nproblem was reported here[1] a couple months ago.\n\nI performed some rudimentary testing of this patch series on Mac OS,\nand it appears to be working as expected; it certainly fixes the\nproblem of git-gui not launching on Mac OS. (I did notice a\nmisbehavior related to the original patch which caused git-gui to be\nunusable on Mac OS in v2.25, but I suspect that misbehavior is not\nrelated to or caused by this patch series, thus shouldn't prevent its\nacceptance.)\n\n[1]: https://github.com/prati0100/git-gui/issues/26\n"},{"id":"393303","messageId":"xmqqwo7k8fnk.fsf@gitster.c.googlers.com","threadId":"52996","inReplyTo":"CAPig+cRXD_bjUL6=daEAx7VnAy_nw9bao6rLK9xwTCYJSk48Qw@mail.gmail.com","subject":"Re: [PATCH v1 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-16T15:48:15Z","receivedAt":"2020-03-16T15:48:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Sat, Mar 14, 2020 at 9:47 PM Pratyush Yadav <me@yadavpratyush.com> wrote:\n>> Some MacOS distributions ship with Tcl 8.5. This means we can't use\n>> TclOO. So, use our homegrown class.tcl instead.\n>\n> It should be mentioned that this patch series fixes a regression in\n> Git v2.25 in which git-gui could not even be launched on Mac OS. The\n> problem was reported here[1] a couple months ago.\n>\n> I performed some rudimentary testing of this patch series on Mac OS,\n> and it appears to be working as expected; it certainly fixes the\n> problem of git-gui not launching on Mac OS. (I did notice a\n> misbehavior related to the original patch which caused git-gui to be\n> unusable on Mac OS in v2.25, but I suspect that misbehavior is not\n> related to or caused by this patch series, thus shouldn't prevent its\n> acceptance.)\n\nI was actually hesitant to see this kind of change for the first\ntime this late in the cycle (the code may work with old Tcl/Tk but\ndo we know it does with newer ones?)  \n\nI'll pull git-gui updates when Pratyush tells me to, which would\nhappen before the final (scheduled on 22nd).  I'll trust git-gui\nmaintainer's decision to include these changes in it, or to cook\nlonger to wait for the 2.27 cycle.  Comments like yours that help\nPratyush make the right judgment is greatly appreciated.\n\nThanks.  \n"},{"id":"393350","messageId":"20200317124902.fitwgrrm6jtv24ec@yadavpratyush.com","threadId":"52996","inReplyTo":"xmqqwo7k8fnk.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v1 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-03-17T12:49:02Z","receivedAt":"2020-03-17T12:49:11Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 16/03/20 08:48AM, Junio C Hamano wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n> \n> > On Sat, Mar 14, 2020 at 9:47 PM Pratyush Yadav <me@yadavpratyush.com> wrote:\n> >> Some MacOS distributions ship with Tcl 8.5. This means we can't use\n> >> TclOO. So, use our homegrown class.tcl instead.\n> >\n> > It should be mentioned that this patch series fixes a regression in\n> > Git v2.25 in which git-gui could not even be launched on Mac OS. The\n> > problem was reported here[1] a couple months ago.\n> >\n> > I performed some rudimentary testing of this patch series on Mac OS,\n> > and it appears to be working as expected; it certainly fixes the\n> > problem of git-gui not launching on Mac OS. (I did notice a\n> > misbehavior related to the original patch which caused git-gui to be\n> > unusable on Mac OS in v2.25, but I suspect that misbehavior is not\n> > related to or caused by this patch series, thus shouldn't prevent its\n> > acceptance.)\n> \n> I was actually hesitant to see this kind of change for the first\n> time this late in the cycle\n\n*sigh* I intended to finish the change much sooner, but one thing after \nanother, and I kept putting it on the backburner :-(\n\n> (the code may work with old Tcl/Tk but do we know it does with newer \n> ones?)  \n\nIt is actually the other way around. My system has Tcl 8.6 installed, \nand it works well with it. The problem is checking if it works with 8.5. \nMy distro's package manager doesn't show Tcl 8.5 as an option (only \n8.6), so I have to try and build 8.5 from source to test it. It is more \nwork than I can afford right now. So I am relying on Eric and other \nMacOS users' reports.\n\nBut IIUC, unless there are any hidden gotchas in Tcl 8.5, there is a \nfairly little chance that this patch breaks anything significant that \nwasn't already broken before. The patch reverts from using a new \n8.6-specific feature to pure, backward-compatible Tcl that should work \nwith older versions too. No, a larger concern for me is whether I missed \nsomething somewhere that breaks the feature for _all_ versions of \ngit-gui.\n \n> I'll pull git-gui updates when Pratyush tells me to, which would\n> happen before the final (scheduled on 22nd).  I'll trust git-gui\n> maintainer's decision to include these changes in it, or to cook\n> longer to wait for the 2.27 cycle.  Comments like yours that help\n> Pratyush make the right judgment is greatly appreciated.\n\nHonestly, I'd like to cook it a bit longer but the reality is that very \nfew people, if any, actually track and test my tree. Most people \nactually discover bugs when the changes hit a new Git release.\n\nI cooked the original series that bumped the version requirement for \nquite some time because I suspected some platforms might still be on \nolder versions. But I got absolutely no feedback/complaints, so I went \nahead and sent you a pull request.\n\nThe change hit my 'master' on December 6. Git v2.25.0 was tagged on \nJanuary 13th. MacOS breakage was reported on January 15th.\n\nSo, I have a choice between waiting for the 2.27 cycle keeping git-gui \nbroken on MacOS for another few months in hopes that someone comes in \nand discovers a bug, or have it merged in v2.26 fixing git-gui for MacOS \nbut risk (though it shouldn't be too high) introducing a bug in _all_ \nplatforms. Neither choice is ideal, but I'm leaning towards the latter. \nHaving _most_ things working is better than having nothing working at \nall.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"393351","messageId":"20200317132921.7222-1-me@yadavpratyush.com","threadId":"52996","inReplyTo":"20200314224159.14174-1-me@yadavpratyush.com","subject":"[PATCH v2 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-03-17T13:29:19Z","receivedAt":"2020-03-17T13:29:37Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi,\n\nSome MacOS distributions ship with Tcl 8.5. This means we can't use\nTclOO. So, use our homegrown class.tcl instead.\n\nWhile here, fix a potential variable name collision by creating a\nseparate namespace for a chord's script evaluation.\n\nJonathan,\n\nCan you please test the patches the same way you tested your original\nseries just to be sure we don't break anything? A review would also be\nnice.\n\nChanges in v2:\n- Add a note _after_ checking if the user agreed to the deletion.\n  Otherwise, if the user denies, two \"zombie\" notes are left lying\n  around which will never be activated. This means that the chord won't\n  complete and the index won't be unlocked, leading to git-gui becoming\n  frozen.\n\nPratyush Yadav (2):\n  git-gui: reduce Tcl version requirement from 8.6 to 8.5\n  git-gui: create a new namespace for chord script evaluation\n\n git-gui.sh    |  4 ++--\n lib/chord.tcl | 56 +++++++++++++++++++++++++--------------------------\n lib/index.tcl | 10 +++++----\n 3 files changed, 35 insertions(+), 35 deletions(-)\n\n--\n2.26.0.rc1.11.g30e9940356\n\n"},{"id":"393352","messageId":"20200317132921.7222-2-me@yadavpratyush.com","threadId":"52996","inReplyTo":"20200317132921.7222-1-me@yadavpratyush.com","subject":"[PATCH v2 1/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-03-17T13:29:20Z","receivedAt":"2020-03-17T13:29:46Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On some MacOS distributions like High Sierra, Tcl 8.5 is shipped by\ndefault. This makes git-gui error out at startup because of the version\nmismatch.\n\nThe only part that requires Tcl 8.6 is SimpleChord, which depends on\nTclOO. So, don't use it and use our homegrown class.tcl instead.\n\nThis means some slight syntax changes. Since class.tcl doesn't have an\n\"unknown\" method like TclOO does, we can't just call '$note', but have\nto use '$note activate' instead. The constructor now needs a proper\nnamespace qualifier. Update the documentation to reflect the new syntax.\n\nAs of now, the only part of git-gui that needs Tcl 8.5 is a call to\n'apply' in lib/index.tcl::lambda. Keep using it until someone shows up\nshouting that their OS ships with 8.4 only. Then we would have to look\ninto implementing it in pure Tcl.\n\nSigned-off-by: Pratyush Yadav <me@yadavpratyush.com>\n---\n git-gui.sh    |  4 ++--\n lib/chord.tcl | 54 ++++++++++++++++++++++++---------------------------\n lib/index.tcl | 10 ++++++----\n 3 files changed, 33 insertions(+), 35 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex f41ed2e..027e093 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -30,8 +30,8 @@ along with this program; if not, see <http://www.gnu.org/licenses/>.}]\n ##\n ## Tcl/Tk sanity check\n \n-if {[catch {package require Tcl 8.6} err]\n- || [catch {package require Tk  8.6} err]\n+if {[catch {package require Tcl 8.5} err]\n+ || [catch {package require Tk  8.5} err]\n } {\n \tcatch {wm withdraw .}\n \ttk_messageBox \\\ndiff --git a/lib/chord.tcl b/lib/chord.tcl\nindex 275a6cd..7de7cba 100644\n--- a/lib/chord.tcl\n+++ b/lib/chord.tcl\n@@ -27,7 +27,7 @@\n #   # Turn off the UI while running a couple of async operations.\n #   lock_ui\n #\n-#   set chord [SimpleChord new {\n+#   set chord [SimpleChord::new {\n #     unlock_ui\n #     # Note: $notice here is not referenced in the calling scope\n #     if {$notice} { info_popup $notice }\n@@ -37,9 +37,9 @@\n #   # all operations have been initiated.\n #   set common_note [$chord add_note]\n #\n-#   # Pass notes as 'after' callbacks to other operations\n-#   async_operation $args [$chord add_note]\n-#   other_async_operation $args [$chord add_note]\n+#   # Activate notes in 'after' callbacks to other operations\n+#   set newnote [$chord add_note]\n+#   async_operation $args [list $newnote activate]\n #\n #   # Communicate with the chord body\n #   if {$condition} {\n@@ -48,7 +48,7 @@\n #   }\n #\n #   # Activate the common note, making the chord eligible to complete\n-#   $common_note\n+#   $common_note activate\n #\n # At this point, the chord will complete at some unknown point in the future.\n # The common note might have been the first note activated, or the async\n@@ -60,18 +60,21 @@\n #   Represents a procedure that conceptually has multiple entrypoints that must\n #   all be called before the procedure executes. Each entrypoint is called a\n #   \"note\". The chord is only \"completed\" when all the notes are \"activated\".\n-oo::class create SimpleChord {\n-\tvariable notes body is_completed\n+class SimpleChord {\n+\tfield notes\n+\tfield body\n+\tfield is_completed\n \n \t# Constructor:\n-\t#   set chord [SimpleChord new {body}]\n+\t#   set chord [SimpleChord::new {body}]\n \t#     Creates a new chord object with the specified body script. The\n \t#     body script is evaluated at most once, when a note is activated\n \t#     and the chord has no other non-activated notes.\n-\tconstructor {body} {\n+\tconstructor new {i_body} {\n \t\tset notes [list]\n-\t\tmy eval [list set body $body]\n+\t\tset body $i_body\n \t\tset is_completed 0\n+\t\treturn $this\n \t}\n \n \t# Method:\n@@ -80,7 +83,7 @@ oo::class create SimpleChord {\n \t#     the chord body will be evaluated. This can be used to set variable\n \t#     values for the chord body to use.\n \tmethod eval {script} {\n-\t\tnamespace eval [self] $script\n+\t\tnamespace eval [namespace qualifiers $this] $script\n \t}\n \n \t# Method:\n@@ -92,7 +95,7 @@ oo::class create SimpleChord {\n \tmethod add_note {} {\n \t\tif {$is_completed} { error \"Cannot add a note to a completed chord\" }\n \n-\t\tset note [ChordNote new [self]]\n+\t\tset note [ChordNote::new $this]\n \n \t\tlappend notes $note\n \n@@ -108,8 +111,8 @@ oo::class create SimpleChord {\n \n \t\t\tset is_completed 1\n \n-\t\t\tnamespace eval [self] $body\n-\t\t\tnamespace delete [self]\n+\t\t\tnamespace eval [namespace qualifiers $this] $body\n+\t\t\tdelete_this\n \t\t}\n \t}\n }\n@@ -119,15 +122,17 @@ oo::class create SimpleChord {\n #   final note of the chord is activated (this can be any note in the chord,\n #   with all other notes already previously activated in any order), the chord's\n #   body is evaluated.\n-oo::class create ChordNote {\n-\tvariable chord is_activated\n+class ChordNote {\n+\tfield chord\n+\tfield is_activated\n \n \t# Constructor:\n \t#   Instances of ChordNote are created internally by calling add_note on\n \t#   SimpleChord objects.\n-\tconstructor {chord} {\n-\t\tmy eval set chord $chord\n+\tconstructor new {c} {\n+\t\tset chord $c\n \t\tset is_activated 0\n+\t\treturn $this\n \t}\n \n \t# Method:\n@@ -138,20 +143,11 @@ oo::class create ChordNote {\n \t}\n \n \t# Method:\n-\t#   $note\n+\t#   $note activate\n \t#     Activates the note, if it has not already been activated, and\n \t#     completes the chord if there are no other notes awaiting\n \t#     activation. Subsequent calls will have no further effect.\n-\t#\n-\t# NB: In TclOO, if an object is invoked like a method without supplying\n-\t#     any method name, then this internal method `unknown` is what\n-\t#     actually runs (with no parameters). It is used in the ChordNote\n-\t#     class for the purpose of allowing the note object to be called as\n-\t#     a function (see example above). (The `unknown` method can also be\n-\t#     used to support dynamic dispatch, but must take parameters to\n-\t#     identify the \"unknown\" method to be invoked. In this form, this\n-\t#     proc serves only to make instances behave directly like methods.)\n-\tmethod unknown {} {\n+\tmethod activate {} {\n \t\tif {!$is_activated} {\n \t\t\tset is_activated 1\n \t\t\t$chord notify_note_activation\ndiff --git a/lib/index.tcl b/lib/index.tcl\nindex 1254145..1fc5b42 100644\n--- a/lib/index.tcl\n+++ b/lib/index.tcl\n@@ -436,7 +436,7 @@ proc revert_helper {txt paths} {\n \t#\n \t# The asynchronous operations are each indicated below by a comment\n \t# before the code block that starts the async operation.\n-\tset after_chord [SimpleChord new {\n+\tset after_chord [SimpleChord::new {\n \t\tif {[string trim $err] != \"\"} {\n \t\t\trescan_on_error $err\n \t\t} else {\n@@ -522,10 +522,11 @@ proc revert_helper {txt paths} {\n \t\t\t]\n \n \t\tif {$reply == 1} {\n+\t\t\tset note [$after_chord add_note]\n \t\t\tcheckout_index \\\n \t\t\t\t$txt \\\n \t\t\t\t$path_list \\\n-\t\t\t\t[$after_chord add_note] \\\n+\t\t\t\t[list $note activate] \\\n \t\t\t\t$capture_error\n \t\t}\n \t}\n@@ -567,14 +568,15 @@ proc revert_helper {txt paths} {\n \t\tif {$reply == 1} {\n \t\t\t$after_chord eval { set should_reshow_diff 1 }\n \n-\t\t\tdelete_files $untracked_list [$after_chord add_note]\n+\t\t\tset note [$after_chord add_note]\n+\t\t\tdelete_files $untracked_list [list $note activate]\n \t\t}\n \t}\n \n \t# Activate the common note. If no other notes were created, this\n \t# completes the chord. If other notes were created, then this common\n \t# note prevents a race condition where the chord might complete early.\n-\t$after_common_note\n+\t$after_common_note activate\n }\n \n # Delete all of the specified files, performing deletion in batches to allow the\n-- \n2.26.0.rc1.11.g30e9940356\n\n"},{"id":"393353","messageId":"20200317132921.7222-3-me@yadavpratyush.com","threadId":"52996","inReplyTo":"20200317132921.7222-1-me@yadavpratyush.com","subject":"[PATCH v2 2/2] git-gui: create a new namespace for chord script evaluation","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-03-17T13:29:21Z","receivedAt":"2020-03-17T13:29:58Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Evaluating the script in the same namespace as the chord itself creates\npotential for variable name collision. And in that case the script would\nunknowingly use the chord's variables.\n\nFor example, say the script has a variable called 'is_completed', which\nalso exists in the chord's namespace. The script then calls 'eval' and\nsets 'is_completed' to 1 thinking it is setting its own variable,\ncompletely unaware of how the chord works behind the scenes. This leads\nto the chord never actually executing because it sees 'is_completed' as\ntrue and thinks it has already completed.\n\nAvoid the potential collision by creating a separate namespace for the\nscript that is a child of the chord's namespace.\n\nSigned-off-by: Pratyush Yadav <me@yadavpratyush.com>\n---\n lib/chord.tcl | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/lib/chord.tcl b/lib/chord.tcl\nindex 7de7cba..e21e7d3 100644\n--- a/lib/chord.tcl\n+++ b/lib/chord.tcl\n@@ -64,6 +64,7 @@ class SimpleChord {\n \tfield notes\n \tfield body\n \tfield is_completed\n+\tfield eval_ns\n \n \t# Constructor:\n \t#   set chord [SimpleChord::new {body}]\n@@ -74,6 +75,7 @@ class SimpleChord {\n \t\tset notes [list]\n \t\tset body $i_body\n \t\tset is_completed 0\n+\t\tset eval_ns \"[namespace qualifiers $this]::eval\"\n \t\treturn $this\n \t}\n \n@@ -83,7 +85,7 @@ class SimpleChord {\n \t#     the chord body will be evaluated. This can be used to set variable\n \t#     values for the chord body to use.\n \tmethod eval {script} {\n-\t\tnamespace eval [namespace qualifiers $this] $script\n+\t\tnamespace eval $eval_ns $script\n \t}\n \n \t# Method:\n@@ -111,7 +113,7 @@ class SimpleChord {\n \n \t\t\tset is_completed 1\n \n-\t\t\tnamespace eval [namespace qualifiers $this] $body\n+\t\t\tnamespace eval $eval_ns $body\n \t\t\tdelete_this\n \t\t}\n \t}\n-- \n2.26.0.rc1.11.g30e9940356\n\n"},{"id":"393456","messageId":"CAPig+cQ0YJB25fFaKV2URz39zdS8BwMwwB-a6VJzekkQRLEHpw@mail.gmail.com","threadId":"52996","inReplyTo":"20200317132921.7222-1-me@yadavpratyush.com","subject":"Re: [PATCH v2 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-19T15:22:05Z","receivedAt":"2020-03-19T15:22:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 17, 2020 at 9:29 AM Pratyush Yadav <me@yadavpratyush.com> wrote:\n> Some MacOS distributions ship with Tcl 8.5. This means we can't use\n> TclOO. So, use our homegrown class.tcl instead.\n>\n> Changes in v2:\n> - Add a note _after_ checking if the user agreed to the deletion.\n>   Otherwise, if the user denies, two \"zombie\" notes are left lying\n>   around which will never be activated. This means that the chord won't\n>   complete and the index won't be unlocked, leading to git-gui becoming\n>   frozen.\n\nThanks. I did some light testing on Mac OS. This re-roll seems to\naddress the reported problems[1] and allows the new \"delete unstaged\nfile\" feature to work on older Tcl. As a fix for the Git 2.25\nregression which resulted in git-gui being unable to launch on Mac OS,\nthis path series seems \"good to go\".\n\n[1]: https://github.com/prati0100/git-gui/issues/26\n"},{"id":"393457","messageId":"CAPig+cRQRD1njsGenUWg-73NF6=krPcDUtZsHNf+jT+0j5JJWQ@mail.gmail.com","threadId":"52996","inReplyTo":"20200317124902.fitwgrrm6jtv24ec@yadavpratyush.com","subject":"Re: [PATCH v1 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-19T15:25:34Z","receivedAt":"2020-03-19T15:25:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 17, 2020 at 8:49 AM Pratyush Yadav <me@yadavpratyush.com> wrote:\n> On 16/03/20 08:48AM, Junio C Hamano wrote:\n> > I'll pull git-gui updates when Pratyush tells me to, which would\n> > happen before the final (scheduled on 22nd).  I'll trust git-gui\n> > maintainer's decision to include these changes in it, or to cook\n> > longer to wait for the 2.27 cycle. [...]\n>\n> Honestly, I'd like to cook it a bit longer but the reality is that very\n> few people, if any, actually track and test my tree. Most people\n> actually discover bugs when the changes hit a new Git release.\n\nYep, that's the big issue. I track Junio's \"next\" branch pretty\nclosely but don't track the git-gui repository at all, so it wasn't\nuntil Junio pulled from you, and after I pulled from Junio, that I\nnoticed the problem. So, in the longer run, asking Junio to pull more\noften -- and earlier -- may be a good way forward.\n"},{"id":"393463","messageId":"20200319160551.ivivebvecw2totnf@yadavpratyush.com","threadId":"52996","inReplyTo":"CAPig+cQ0YJB25fFaKV2URz39zdS8BwMwwB-a6VJzekkQRLEHpw@mail.gmail.com","subject":"Re: [PATCH v2 0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-03-19T16:05:51Z","receivedAt":"2020-03-19T16:05:58Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 19/03/20 11:22AM, Eric Sunshine wrote:\n> On Tue, Mar 17, 2020 at 9:29 AM Pratyush Yadav <me@yadavpratyush.com> wrote:\n> > Some MacOS distributions ship with Tcl 8.5. This means we can't use\n> > TclOO. So, use our homegrown class.tcl instead.\n> >\n> > Changes in v2:\n> > - Add a note _after_ checking if the user agreed to the deletion.\n> >   Otherwise, if the user denies, two \"zombie\" notes are left lying\n> >   around which will never be activated. This means that the chord won't\n> >   complete and the index won't be unlocked, leading to git-gui becoming\n> >   frozen.\n> \n> Thanks. I did some light testing on Mac OS. This re-roll seems to\n> address the reported problems[1] and allows the new \"delete unstaged\n> file\" feature to work on older Tcl. As a fix for the Git 2.25\n> regression which resulted in git-gui being unable to launch on Mac OS,\n> this path series seems \"good to go\".\n\nThanks for testing. Merged. Thanks all.\n \n> [1]: https://github.com/prati0100/git-gui/issues/26\n\n-- \nRegards,\nPratyush Yadav\n"}]}