{"thread":{"id":"53200","subject":"[PATCH] git-gui: fix concatenation of ui_ready by adding \";\"","startedAt":"2020-04-09T18:03:30Z","lastAt":"2020-04-22T13:07:21Z","messageCount":2,"participants":["Ansgar Röber via GitGitGadget","Pratyush Yadav"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"395134","messageId":"pull.607.git.1586455405590.gitgitgadget@gmail.com","threadId":"53200","inReplyTo":null,"subject":"[PATCH] git-gui: fix concatenation of ui_ready by adding \";\"","fromName":"Ansgar Röber via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-09T18:03:25Z","receivedAt":"2020-04-09T18:03:30Z","isPatch":true,"sender":{"key":"name:Ansgar Röber","avatar":null},"body":"From: =?UTF-8?q?Ansgar=20R=C3=B6ber?= <ansgar.roeber@rwth-aachen.de>\n\nThis fixes https://github.com/git-for-windows/git/issues/2469\nSigned-off-by: Ansgar Röber <ansgar.roeber@rwth-aachen.de>\n---\n    git-gui: fix concatenation of ui_ready by adding \";\"\n    \n    This fixes https://github.com/git-for-windows/git/issues/2469\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-607%2FIsengart%2Ffix-stage-to-commit-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-607/Isengart/fix-stage-to-commit-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/607\n\n git-gui.sh        | 4 ++--\n lib/index.tcl     | 4 ++--\n lib/mergetool.tcl | 2 +-\n 3 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 4610e4ca72a..faaf93b431a 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2600,12 +2600,12 @@ proc toggle_or_diff {mode w args} {\n \t\t\tupdate_indexinfo \\\n \t\t\t\t\"Unstaging [short_path $path] from commit\" \\\n \t\t\t\t[list $path] \\\n-\t\t\t\t[concat $after [list ui_ready]]\n+\t\t\t\t[concat $after {ui_ready;}]\n \t\t} elseif {$w eq $ui_workdir} {\n \t\t\tupdate_index \\\n \t\t\t\t\"Adding [short_path $path]\" \\\n \t\t\t\t[list $path] \\\n-\t\t\t\t[concat $after [list ui_ready]]\n+\t\t\t\t[concat $after {ui_ready;}]\n \t\t}\n \t} else {\n \t\tset selected_paths($path) 1\ndiff --git a/lib/index.tcl b/lib/index.tcl\nindex 1fc5b42300d..59d1f7542a8 100644\n--- a/lib/index.tcl\n+++ b/lib/index.tcl\n@@ -60,7 +60,7 @@ proc rescan_on_error {err {after {}}} {\n \n \t$::main_status stop_all\n \tunlock_index\n-\trescan [concat $after [list ui_ready]] 0\n+\trescan [concat $after {ui_ready;}] 0\n }\n \n proc update_indexinfo {msg path_list after} {\n@@ -314,7 +314,7 @@ proc unstage_helper {txt paths} {\n \t\tupdate_indexinfo \\\n \t\t\t$txt \\\n \t\t\t$path_list \\\n-\t\t\t[concat $after [list ui_ready]]\n+\t\t\t[concat $after {ui_ready;}]\n \t}\n }\n \ndiff --git a/lib/mergetool.tcl b/lib/mergetool.tcl\nindex 120bc4064b6..e688b016ef6 100644\n--- a/lib/mergetool.tcl\n+++ b/lib/mergetool.tcl\n@@ -59,7 +59,7 @@ proc merge_add_resolution {path} {\n \tupdate_index \\\n \t\t[mc \"Adding resolution for %s\" [short_path $path]] \\\n \t\t[list $path] \\\n-\t\t[concat $after [list ui_ready]]\n+\t\t[concat $after {ui_ready;}]\n }\n \n proc merge_force_stage {stage} {\n\nbase-commit: a5728022e07c53e5ac91db0960870518e243b7c1\n-- \ngitgitgadget\n"},{"id":"395898","messageId":"20200422130710.stwob3bniyj2wvm3@yadavpratyush.com","threadId":"53200","inReplyTo":"pull.607.git.1586455405590.gitgitgadget@gmail.com","subject":"Re: [PATCH] git-gui: fix concatenation of ui_ready by adding \";\"","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2020-04-22T13:07:10Z","receivedAt":"2020-04-22T13:07:21Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Ansgar,\n\nSorry for the late reply. Thanks for the patch. The code changes all \nlook correct, other than one extra hunk I suggest (see below). The \ncommit subject and message not so much.\n\n> Subject: [PATCH] git-gui: fix concatenation of ui_ready by adding \";\"\n\nWe aren't fixing concatenation of \"ui_ready\". We are fixing a syntax \nerror because we didn't add a statement separator (semicolon), and so if \nwe later append something else, it would be treated as an argument to \nthe ui_ready call by Tcl, causing a syntax error.\n\nOn 09/04/20 06:03PM, Ansgar Röber via GitGitGadget wrote:\n> From: =?UTF-8?q?Ansgar=20R=C3=B6ber?= <ansgar.roeber@rwth-aachen.de>\n> \n> This fixes https://github.com/git-for-windows/git/issues/2469\n\nInstead of linking to another project's GitHub, describe the problem and \nits solution in the commit message.\n\n> Signed-off-by: Ansgar Röber <ansgar.roeber@rwth-aachen.de>\n\nSo, based on the suggestions, I propose the following commit message:\n\n  Subject: git-gui: fix syntax error because of missing semicolon\n  \n  For some asynchronous operations, we build a chain of callbacks to \n  execute when the operation is done. These callbacks are held in $after, \n  and a new callback can be added by appending to $after. Once the \n  operation is done, $after is executed as a script.\n  \n  But if we don't append a semi-colon after the procedure calls, they will \n  appear to Tcl as arguments to the previous procedure's arguments. So, \n  for example, if $after is \"foo\", and we just append \"bar\", then $after \n  becomes \"foo bar\", and bar will be treated as an argument to foo. If foo \n  does not accept any optional arguments, it would result in Tcl throwing \n  an error. If instead we do append a semi-colon, $after will look like \n  \"foo;bar;\", and these will be treated as two separate procedure calls.\n  \n  Before d9c6469 (git-gui: update status bar to track operations, \n  2019-12-01), this problem was masked because ui_ready/ui_status did \n  accept an optional argument. In d9c6469, ui_ready stopped accepting an \n  optional argument, and this error started showing up.\n  \n  Another instance of this problem is when a call to ui_status without a \n  trailing semicolon. ui_status never accepted an optional argument to \n  begin with, but the issue never managed to surface.\n  \n  So, fix these errors by making sure we always append a semi-colon after \n  procedure calls when multiple callbacks are involved in $after.\n\n> ---\n>  git-gui.sh        | 4 ++--\n>  lib/index.tcl     | 4 ++--\n>  lib/mergetool.tcl | 2 +-\n>  3 files changed, 5 insertions(+), 5 deletions(-)\n> \n> diff --git a/git-gui.sh b/git-gui.sh\n> index 4610e4ca72a..faaf93b431a 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> @@ -2600,12 +2600,12 @@ proc toggle_or_diff {mode w args} {\n>  \t\t\tupdate_indexinfo \\\n>  \t\t\t\t\"Unstaging [short_path $path] from commit\" \\\n>  \t\t\t\t[list $path] \\\n> -\t\t\t\t[concat $after [list ui_ready]]\n> +\t\t\t\t[concat $after {ui_ready;}]\n>  \t\t} elseif {$w eq $ui_workdir} {\n>  \t\t\tupdate_index \\\n>  \t\t\t\t\"Adding [short_path $path]\" \\\n>  \t\t\t\t[list $path] \\\n> -\t\t\t\t[concat $after [list ui_ready]]\n> +\t\t\t\t[concat $after {ui_ready;}]\n>  \t\t}\n>  \t} else {\n>  \t\tset selected_paths($path) 1\n> diff --git a/lib/index.tcl b/lib/index.tcl\n> index 1fc5b42300d..59d1f7542a8 100644\n> --- a/lib/index.tcl\n> +++ b/lib/index.tcl\n> @@ -60,7 +60,7 @@ proc rescan_on_error {err {after {}}} {\n>  \n>  \t$::main_status stop_all\n>  \tunlock_index\n> -\trescan [concat $after [list ui_ready]] 0\n> +\trescan [concat $after {ui_ready;}] 0\n>  }\n>  \n>  proc update_indexinfo {msg path_list after} {\n> @@ -314,7 +314,7 @@ proc unstage_helper {txt paths} {\n>  \t\tupdate_indexinfo \\\n>  \t\t\t$txt \\\n>  \t\t\t$path_list \\\n> -\t\t\t[concat $after [list ui_ready]]\n> +\t\t\t[concat $after {ui_ready;}]\n>  \t}\n>  }\n  \nAnother hunk I think we should add here is:\n\n  @@ -366,7 +366,7 @@ proc add_helper {txt paths} {\n   \t\tupdate_index \\\n   \t\t\t$txt \\\n   \t\t\t$path_list \\\n  -\t\t\t[concat $after {ui_status [mc \"Ready to commit.\"]}]\n  +\t\t\t[concat $after {ui_status [mc \"Ready to commit.\"];}]\n   \t}\n   }\n\nI've changed the commit message and added this hunk locally, so no need \nto send another version. You can find the final commit here [0]. If you \nhave any comments/suggestions, please let me know. Otherwise, I'll merge \nit in a few days.\n\n[0] https://github.com/prati0100/git-gui/commit/19195fbd73994d05abaa0a2976143da96b320f47\n\n-- \nRegards,\nPratyush Yadav\n"}]}