{"thread":{"id":"52142","subject":"[PATCH 0/1] Allow the 'revert' option in Git Gui to operate on untracked files, deleting them","startedAt":"2019-10-28T18:58:13Z","lastAt":"2019-10-29T23:52:55Z","messageCount":10,"participants":["Jonathan Gilbert via GitGitGadget","brian m. carlson","Jonathan Gilbert","Bert Wesarg","Pratyush Yadav"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"385010","messageId":"pull.423.git.1572289087.gitgitgadget@gmail.com","threadId":"52142","inReplyTo":null,"subject":"[PATCH 0/1] Allow the 'revert' option in Git Gui to operate on untracked files, deleting them","fromName":"Jonathan Gilbert via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-28T18:58:06Z","receivedAt":"2019-10-28T18:58:13Z","isPatch":true,"sender":{"key":"logic@deltaq.org","avatar":"https://gravatar.com/avatar/6aa0c0a07ad7988ae46c3c5319d4bbdaa73773b49fb7e668c4bb0aad34b8f67f?d=mp&s=160"},"body":"My development environment sometimes makes automatic changes that I don't\nwant to keep. In some cases, this involves new files being added that I\ndon't want to commit or keep. I have typically had to explicitly delete\nthose files externally to Git Gui, and I want to be able to just select\nthose newly-created untracked files and \"revert\" them into oblivion.\n\nThis change updates the revert_helper function to check for untracked files\nas well as changes, and then any changes to be reverted and untracked files\nare handled by independent blocks of code. The user is prompted\nindependently for untracked files, since the underlying action is\nfundamentally different (rm -f). If after deleting untracked files, the\ndirectory containing them becomes empty, then the directory is removed as\nwell.\n\nThis introduces new strings in index.tcl. I have been told that there is a\nseparate process whereby the translations get updated.\n\nJonathan Gilbert (1):\n  git-gui: Revert untracked files by deleting them\n\n git-gui/lib/index.tcl | 139 +++++++++++++++++++++++++++++++-----------\n 1 file changed, 104 insertions(+), 35 deletions(-)\n\n\nbase-commit: 566a1439f6f56c2171b8853ddbca0ad3f5098770\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-423%2Flogiclrd%2Fgit-gui-revert-untracked-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-423/logiclrd/git-gui-revert-untracked-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/423\n-- \ngitgitgadget\n"},{"id":"385011","messageId":"56faba848676ffc5441f373bc2105392fc2a3e11.1572289087.git.gitgitgadget@gmail.com","threadId":"52142","inReplyTo":"pull.423.git.1572289087.gitgitgadget@gmail.com","subject":"[PATCH 1/1] git-gui: Revert untracked files by deleting them","fromName":"Jonathan Gilbert via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-28T18:58:07Z","receivedAt":"2019-10-28T18:58:14Z","isPatch":true,"sender":{"key":"logic@deltaq.org","avatar":"https://gravatar.com/avatar/6aa0c0a07ad7988ae46c3c5319d4bbdaa73773b49fb7e668c4bb0aad34b8f67f?d=mp&s=160"},"body":"From: Jonathan Gilbert <JonathanG@iQmetrix.com>\n\nMy development environment sometimes makes automatic changes\nthat I don't want to keep. In some cases, this involves new\nfiles being added that I don't want to commit or keep. I have\ntypically had to explicitly delete those files externally to\nGit Gui, and I want to be able to just select those newly-\ncreated untracked files and \"revert\" them into oblivion.\n\nThis change updates the revert_helper function to check for\nuntracked files as well as changes, and then any changes to be\nreverted and untracked files are handled by independent\nblocks of code. The user is prompted independently for\nuntracked files, since the underlying action is fundamentally\ndifferent (rm -f). If after deleting untracked files, the\ndirectory containing them becomes empty, then the directory is\nremoved as well.\n\nThis introduces new strings in index.tcl. I have been told that\nthere is a separate process whereby the translations get updated.\n\nSigned-off-by: Jonathan Gilbert <JonathanG@iQmetrix.com>\n---\n git-gui/lib/index.tcl | 139 +++++++++++++++++++++++++++++++-----------\n 1 file changed, 104 insertions(+), 35 deletions(-)\n\ndiff --git a/git-gui/lib/index.tcl b/git-gui/lib/index.tcl\nindex e07b7a3762..cc1b651d56 100644\n--- a/git-gui/lib/index.tcl\n+++ b/git-gui/lib/index.tcl\n@@ -393,11 +393,24 @@ proc revert_helper {txt paths} {\n \n \tif {![lock_index begin-update]} return\n \n+\t# If an action is taken that implicitly unlocks the index, this gets cleared. Either way, it is executed at the end of the procedure.\n+\tset epilogue [list]\n+\tlappend epilogue {unlock_index}\n+\n+\tproc already_unlocked {} { upvar epilogue epilogue; set epilogue [lsearch -inline -all -not -exact $epilogue {unlock_index}] }\n+\n \tset pathList [list]\n+\tset untrackedList [list]\n \tset after {}\n \tforeach path $paths {\n \t\tswitch -glob -- [lindex $file_states($path) 0] {\n \t\tU? {continue}\n+\t\t?O {\n+\t\t\tlappend untrackedList $path\n+\t\t\tif {$path eq $current_diff_path} {\n+\t\t\t\tset after {reshow_diff;}\n+\t\t\t}\n+\t\t}\n \t\t?M -\n \t\t?T -\n \t\t?D {\n@@ -410,45 +423,101 @@ proc revert_helper {txt paths} {\n \t}\n \n \n-\t# Split question between singular and plural cases, because\n-\t# such distinction is needed in some languages. Previously, the\n-\t# code used \"Revert changes in\" for both, but that can't work\n-\t# in languages where 'in' must be combined with word from\n-\t# rest of string (in different way for both cases of course).\n-\t#\n-\t# FIXME: Unfortunately, even that isn't enough in some languages\n-\t# as they have quite complex plural-form rules. Unfortunately,\n-\t# msgcat doesn't seem to support that kind of string translation.\n-\t#\n-\tset n [llength $pathList]\n-\tif {$n == 0} {\n-\t\tunlock_index\n-\t\treturn\n-\t} elseif {$n == 1} {\n-\t\tset query [mc \"Revert changes in file %s?\" [short_path [lindex $pathList]]]\n-\t} else {\n-\t\tset query [mc \"Revert changes in these %i files?\" $n]\n-\t}\n+\tset numPaths [llength $pathList]\n+\tset numUntracked [llength $untrackedList]\n \n-\tset reply [tk_dialog \\\n-\t\t.confirm_revert \\\n-\t\t\"[appname] ([reponame])\" \\\n-\t\t\"$query\n+\tif {$numPaths > 0} {\n+\t\t# Split question between singular and plural cases, because\n+\t\t# such distinction is needed in some languages. Previously, the\n+\t\t# code used \"Revert changes in\" for both, but that can't work\n+\t\t# in languages where 'in' must be combined with word from\n+\t\t# rest of string (in different way for both cases of course).\n+\t\t#\n+\t\t# FIXME: Unfortunately, even that isn't enough in some languages\n+\t\t# as they have quite complex plural-form rules. Unfortunately,\n+\t\t# msgcat doesn't seem to support that kind of string translation.\n+\t\tif {$numPaths == 1} {\n+\t\t\tset query [mc \"Revert changes in file %s?\" [short_path [lindex $pathList]]]\n+\t\t} else {\n+\t\t\tset query [mc \"Revert changes in these %i files?\" $numPaths]\n+\t\t}\n+\n+\t\tset reply [tk_dialog \\\n+\t\t\t.confirm_revert \\\n+\t\t\t\"[appname] ([reponame])\" \\\n+\t\t\t\"$query\n \n [mc \"Any unstaged changes will be permanently lost by the revert.\"]\" \\\n-\t\tquestion \\\n-\t\t1 \\\n-\t\t[mc \"Do Nothing\"] \\\n-\t\t[mc \"Revert Changes\"] \\\n-\t\t]\n-\tif {$reply == 1} {\n-\t\tcheckout_index \\\n-\t\t\t$txt \\\n-\t\t\t$pathList \\\n-\t\t\t[concat $after [list ui_ready]]\n-\t} else {\n-\t\tunlock_index\n+\t\t\tquestion \\\n+\t\t\t1 \\\n+\t\t\t[mc \"Do Nothing\"] \\\n+\t\t\t[mc \"Revert Changes\"] \\\n+\t\t\t]\n+\n+\t\tif {$reply == 1} {\n+\t\t\tcheckout_index \\\n+\t\t\t\t$txt \\\n+\t\t\t\t$pathList \\\n+\t\t\t\t[concat $after [list ui_ready]]\n+\n+\t\t\talready_unlocked\n+\t\t}\n+\t}\n+\n+\tif {$numUntracked > 0} {\n+\t\t# Split question between singular and plural cases, because\n+\t\t# such distinction is needed in some languages.\n+\t\t#\n+\t\t# FIXME: Unfortunately, even that isn't enough in some languages\n+\t\t# as they have quite complex plural-form rules. Unfortunately,\n+\t\t# msgcat doesn't seem to support that kind of string translation.\n+\t\tif {$numUntracked == 1} {\n+\t\t\tset query [mc \"Delete untracked file %s?\" [short_path [lindex $untrackedList]]]\n+\t\t} else {\n+\t\t\tset query [mc \"Delete these %i untracked files?\" $numUntracked]\n+\t\t}\n+\n+\t\tset reply [tk_dialog \\\n+\t\t\t.confirm_revert \\\n+\t\t\t\"[appname] ([reponame])\" \\\n+\t\t\t\"$query\n+\n+[mc \"Files will be permanently deleted.\"]\" \\\n+\t\t\tquestion \\\n+\t\t\t1 \\\n+\t\t\t[mc \"Do Nothing\"] \\\n+\t\t\t[mc \"Delete Files\"] \\\n+\t\t\t]\n+\n+\t\tif {$reply == 1} {\n+\t\t\tfile delete -- {*}$untrackedList\n+\n+\t\t\tforeach path $untrackedList {\n+\t\t\t\tset directoryPath [file dirname $path]\n+\n+\t\t\t\twhile {$directoryPath != $path} {\n+\t\t\t\t\tset contents [glob -nocomplain -dir $path *]\n+\n+\t\t\t\t\tif {[llength $contents] > 0} { break }\n+\n+\t\t\t\t\ttry {\n+\t\t\t\t\t\tfile delete -- $path\n+\t\t\t\t\t}\n+\t\t\t\t\tcatch {\n+\t\t\t\t\t\t# This is just a best effort, don't annoy the user with failure to remove empty directories.\n+\t\t\t\t\t\tbreak\n+\t\t\t\t\t}\n+\n+\t\t\t\t\tset path $directoryPath\n+\t\t\t\t\tset directoryPath [file dirname $path]\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\tlappend epilogue {ui_do_rescan}\n+\t\t}\n \t}\n+\n+\tforeach epilogueCommand $epilogue { {*}$epilogueCommand }\n }\n \n proc do_revert_selection {} {\n-- \ngitgitgadget\n"},{"id":"385027","messageId":"20191029001253.2lxsntgcivs2jhax@camp.crustytoothpaste.net","threadId":"52142","inReplyTo":"pull.423.git.1572289087.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] Allow the 'revert' option in Git Gui to operate on untracked files, deleting them","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-10-29T00:12:53Z","receivedAt":"2019-10-29T00:13:05Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-10-28 at 18:58:06, Jonathan Gilbert via GitGitGadget wrote:\n> My development environment sometimes makes automatic changes that I don't\n> want to keep. In some cases, this involves new files being added that I\n> don't want to commit or keep. I have typically had to explicitly delete\n> those files externally to Git Gui, and I want to be able to just select\n> those newly-created untracked files and \"revert\" them into oblivion.\n\nIs there a reason these new files can't be ignored, with one of the\n.gitignore file, .git/info/exclude, or core.excludesFile?\n\nIf so, it would be helpful to explain that in the commit message so we\ncan more fully understand the rationale here.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"385033","messageId":"CAPSOpYu4pHymWpSrph41JsbR00dvpFUVrSPAx3S+RqRUi8t+ig@mail.gmail.com","threadId":"52142","inReplyTo":"20191029001253.2lxsntgcivs2jhax@camp.crustytoothpaste.net","subject":"Re: [PATCH 0/1] Allow the 'revert' option in Git Gui to operate on untracked files, deleting them","fromName":"Jonathan Gilbert","fromEmail":"logic@deltaq.org","sentAt":"2019-10-29T01:45:08Z","receivedAt":"2019-10-29T01:45:32Z","isPatch":true,"sender":{"key":"logic@deltaq.org","avatar":"https://gravatar.com/avatar/6aa0c0a07ad7988ae46c3c5319d4bbdaa73773b49fb7e668c4bb0aad34b8f67f?d=mp&s=160"},"body":"> Is there a reason these new files can't be ignored, with one of the.gitignore file, .git/info/exclude, or core.excludesFile?\n\nI guess it's implied in the way I worded the message, but I have\nfallen into the habit of using Git-Gui to manage the state of the\nworking copy (at least in detail). I am primarily doing .NET\ndevelopment, for which projects can have a file App.config that stores\nvarious settings that apply to the project at runtime. The NuGet\npackage manager sometimes edits App.config on your behalf as part of\ninstalling a package, and if a project doesn't already have an\nApp.config file, it adds one. It has also updated a packages.config\nfile and the main project file. If I decide that I actually don't want\nthe change after all, Git-Gui permits me to revert the packages.config\nchange and revert the project file change. If an existing App.config\nfile was edited, I can revert that too, but if it was newly-generated,\nthen I want to delete that file, but I don't want to ignore App.config\nfiles going forward, because there's a good chance a future change may\nintroduce a different need for an App.config file.\n\nWith the current Git-Gui version, I need to exit the Git-Gui UI/flow,\nnavigate to the project in a console window (which probably isn't\nalready in the correct folder) and manually delete the unwanted file.\nThis deletion requires me to identify the file explicitly as well.\nWith the proposed change, the untracked file, which Git-Gui already\nlists, can be selected, and then activating the \"revert\" function\nperforms a UI flow for deleting the file. Without these changes,\nGit-Gui simply does nothing at all when you tell it to revert an\nuntracked file.\n\nAnother example is when I go to review changes and discover a VIM\ncrash dump or a spurious temporary file from a code analyzer, files\nthat aren't _expected_ to normally come into existence at all. While\nwe could try to anticipate every type of spurious file and .gitignore\nthem all, I prefer to simply delete the files, and for similar reasons\nas before, navigating a separate tool to the correct folder to perform\nthe deletion is a manual, time-consuming and context-switching\nprocess. This is what led me to want this feature directly from the\nGit-Gui tool, which is what identifies the rogue file to me in the\nfirst place.\n\nIf you can think of a concise way to say that, I'll be happy to add it\nto the commit message. My intuition is that that's too wordy\nas-written, but if that intuition is wrong, I can copy/paste this text\ntoo. :-)\n\nThanks,\n\nJonathan Gilbert\n\n\nOn Mon, Oct 28, 2019 at 7:12 PM brian m. carlson\nsandals-at-crustytoothpaste.net |GitHub Public/Example Allow|\n<92ue75mvem3o2ht@sneakemail.com> wrote:\n>\n> On 2019-10-28 at 18:58:06, Jonathan Gilbert via GitGitGadget wrote:\n> > My development environment sometimes makes automatic changes that I don't\n> > want to keep. In some cases, this involves new files being added that I\n> > don't want to commit or keep. I have typically had to explicitly delete\n> > those files externally to Git Gui, and I want to be able to just select\n> > those newly-created untracked files and \"revert\" them into oblivion.\n>\n> Is there a reason these new files can't be ignored, with one of the\n> .gitignore file, .git/info/exclude, or core.excludesFile?\n>\n> If so, it would be helpful to explain that in the commit message so we\n> can more fully understand the rationale here.\n> --\n> brian m. carlson: Houston, Texas, US\n> OpenPGP: https://keybase.io/bk2204\n"},{"id":"385087","messageId":"CAKPyHN3WEpVLnxYN5FsovW3G_borc+YrF05Npr0Wyvv6YhbcaQ@mail.gmail.com","threadId":"52142","inReplyTo":"pull.423.git.1572289087.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] Allow the 'revert' option in Git Gui to operate on untracked files, deleting them","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-10-29T14:29:00Z","receivedAt":"2019-10-29T14:29:15Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, Oct 28, 2019 at 7:58 PM Jonathan Gilbert via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> My development environment sometimes makes automatic changes that I don't\n> want to keep. In some cases, this involves new files being added that I\n> don't want to commit or keep. I have typically had to explicitly delete\n> those files externally to Git Gui, and I want to be able to just select\n> those newly-created untracked files and \"revert\" them into oblivion.\n>\n\nI have an entry in the 'Tools\" menu for this called 'Delete':\n\n[guitool \"Delete\"]\n    cmd = rm -f \\\"$FILENAME\\\"\n    noconsole = yes\n    needsfile = yes\n    confirm = yes\n\nBest,\nBert\n\n> This change updates the revert_helper function to check for untracked files\n> as well as changes, and then any changes to be reverted and untracked files\n> are handled by independent blocks of code. The user is prompted\n> independently for untracked files, since the underlying action is\n> fundamentally different (rm -f). If after deleting untracked files, the\n> directory containing them becomes empty, then the directory is removed as\n> well.\n>\n> This introduces new strings in index.tcl. I have been told that there is a\n> separate process whereby the translations get updated.\n>\n> Jonathan Gilbert (1):\n>   git-gui: Revert untracked files by deleting them\n>\n>  git-gui/lib/index.tcl | 139 +++++++++++++++++++++++++++++++-----------\n>  1 file changed, 104 insertions(+), 35 deletions(-)\n>\n>\n"},{"id":"385106","messageId":"CAPSOpYsA9n6Y1vH1_+vRTCV7Mrvox4L2LpQmpXBnpH8ke0yDhQ@mail.gmail.com","threadId":"52142","inReplyTo":"CAKPyHN3WEpVLnxYN5FsovW3G_borc+YrF05Npr0Wyvv6YhbcaQ@mail.gmail.com","subject":"Re: [PATCH 0/1] Allow the 'revert' option in Git Gui to operate on untracked files, deleting them","fromName":"Jonathan Gilbert","fromEmail":"logic@deltaq.org","sentAt":"2019-10-29T20:25:04Z","receivedAt":"2019-10-29T20:25:29Z","isPatch":true,"sender":{"key":"logic@deltaq.org","avatar":"https://gravatar.com/avatar/6aa0c0a07ad7988ae46c3c5319d4bbdaa73773b49fb7e668c4bb0aad34b8f67f?d=mp&s=160"},"body":"That's kind of neat, I wasn't aware of that facet of Git Gui :-) But,\nit isn't quite the same feature:\n\n* It has to be manually set up on each installation.\n* It invokes an external process, I don't know if it's safe to assume\nthat \"rm\" will work on all platforms (though I just tested it on my\nWindows installation and it worked).\n* It doesn't remove directories that it makes empty.\n* I don't see a way to bind it to a keyboard shortcut. That could just\nbe me not knowing enough about custom tools, though. :-)\n* It only processes the first file selected.\n* If I select a tracked file, it will still delete it, and the feature\nI'm looking for is more of a \"return repository to clean state\" type\nfunction, like \"revert\" already is but extended to handle files that\nyou can't actually \"git revert\".\n\nThanks,\n\nJonathan Gilbert\n\nOn Tue, Oct 29, 2019 at 9:32 AM Bert Wesarg\nbert.wesarg-at-googlemail.com |GitHub Public/Example Allow|\n<xlwsizdz58ciy7t@sneakemail.com> wrote:\n>\n> On Mon, Oct 28, 2019 at 7:58 PM Jonathan Gilbert via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> >\n> > My development environment sometimes makes automatic changes that I don't\n> > want to keep. In some cases, this involves new files being added that I\n> > don't want to commit or keep. I have typically had to explicitly delete\n> > those files externally to Git Gui, and I want to be able to just select\n> > those newly-created untracked files and \"revert\" them into oblivion.\n> >\n>\n> I have an entry in the 'Tools\" menu for this called 'Delete':\n>\n> [guitool \"Delete\"]\n>     cmd = rm -f \\\"$FILENAME\\\"\n>     noconsole = yes\n>     needsfile = yes\n>     confirm = yes\n>\n> Best,\n> Bert\n>\n> > This change updates the revert_helper function to check for untracked files\n> > as well as changes, and then any changes to be reverted and untracked files\n> > are handled by independent blocks of code. The user is prompted\n> > independently for untracked files, since the underlying action is\n> > fundamentally different (rm -f). If after deleting untracked files, the\n> > directory containing them becomes empty, then the directory is removed as\n> > well.\n> >\n> > This introduces new strings in index.tcl. I have been told that there is a\n> > separate process whereby the translations get updated.\n> >\n> > Jonathan Gilbert (1):\n> >   git-gui: Revert untracked files by deleting them\n> >\n> >  git-gui/lib/index.tcl | 139 +++++++++++++++++++++++++++++++-----------\n> >  1 file changed, 104 insertions(+), 35 deletions(-)\n> >\n> >\n"},{"id":"385109","messageId":"CAPSOpYs1zqbbbsAD=cNvQ6M_4EvziBaf6B=-BfS7XoZhU5cD_Q@mail.gmail.com","threadId":"52142","inReplyTo":"CAPSOpYsA9n6Y1vH1_+vRTCV7Mrvox4L2LpQmpXBnpH8ke0yDhQ@mail.gmail.com","subject":"Re: [PATCH 0/1] Allow the 'revert' option in Git Gui to operate on untracked files, deleting them","fromName":"Jonathan Gilbert","fromEmail":"logic@deltaq.org","sentAt":"2019-10-29T20:33:46Z","receivedAt":"2019-10-29T20:34:12Z","isPatch":true,"sender":{"key":"logic@deltaq.org","avatar":"https://gravatar.com/avatar/6aa0c0a07ad7988ae46c3c5319d4bbdaa73773b49fb7e668c4bb0aad34b8f67f?d=mp&s=160"},"body":"(should have had:)\n> > I have an entry in the \"Tools\" menu for this called 'Delete':\n\n> That's kind of neat, I wasn't aware of that facet of Git Gui :-) But,\n> it isn't quite the same feature:\n\nOops, double gaffe. I accidentally forgot to \"Reply All\", so this was\na re-send of the message. And when I re-sent it, I didn't notice that\nthe e-mail client hid the quoted line from me and accidentally sent it\nwithout quoting Bert's line. I wasn't sure whether to write this\nfollow-up but the longer I stared at it, the more sure I was that\nsomebody would call me out on it so I decided to be pre-emptive. My\napologies if it would have been better to just let it slide.\n\nJonathan Gilbert\n\nOn Tue, Oct 29, 2019 at 3:25 PM Jonathan Gilbert <logic@deltaq.org> wrote:\n>\n> That's kind of neat, I wasn't aware of that facet of Git Gui :-) But,\n> it isn't quite the same feature:\n>\n> * It has to be manually set up on each installation.\n> * It invokes an external process, I don't know if it's safe to assume\n> that \"rm\" will work on all platforms (though I just tested it on my\n> Windows installation and it worked).\n> * It doesn't remove directories that it makes empty.\n> * I don't see a way to bind it to a keyboard shortcut. That could just\n> be me not knowing enough about custom tools, though. :-)\n> * It only processes the first file selected.\n> * If I select a tracked file, it will still delete it, and the feature\n> I'm looking for is more of a \"return repository to clean state\" type\n> function, like \"revert\" already is but extended to handle files that\n> you can't actually \"git revert\".\n>\n> Thanks,\n>\n> Jonathan Gilbert\n>\n> On Tue, Oct 29, 2019 at 9:32 AM Bert Wesarg\n> bert.wesarg-at-googlemail.com |GitHub Public/Example Allow|\n> <xlwsizdz58ciy7t@sneakemail.com> wrote:\n> >\n> > On Mon, Oct 28, 2019 at 7:58 PM Jonathan Gilbert via GitGitGadget\n> > <gitgitgadget@gmail.com> wrote:\n> > >\n> > > My development environment sometimes makes automatic changes that I don't\n> > > want to keep. In some cases, this involves new files being added that I\n> > > don't want to commit or keep. I have typically had to explicitly delete\n> > > those files externally to Git Gui, and I want to be able to just select\n> > > those newly-created untracked files and \"revert\" them into oblivion.\n> > >\n> >\n> > I have an entry in the 'Tools\" menu for this called 'Delete':\n> >\n> > [guitool \"Delete\"]\n> >     cmd = rm -f \\\"$FILENAME\\\"\n> >     noconsole = yes\n> >     needsfile = yes\n> >     confirm = yes\n> >\n> > Best,\n> > Bert\n> >\n> > > This change updates the revert_helper function to check for untracked files\n> > > as well as changes, and then any changes to be reverted and untracked files\n> > > are handled by independent blocks of code. The user is prompted\n> > > independently for untracked files, since the underlying action is\n> > > fundamentally different (rm -f). If after deleting untracked files, the\n> > > directory containing them becomes empty, then the directory is removed as\n> > > well.\n> > >\n> > > This introduces new strings in index.tcl. I have been told that there is a\n> > > separate process whereby the translations get updated.\n> > >\n> > > Jonathan Gilbert (1):\n> > >   git-gui: Revert untracked files by deleting them\n> > >\n> > >  git-gui/lib/index.tcl | 139 +++++++++++++++++++++++++++++++-----------\n> > >  1 file changed, 104 insertions(+), 35 deletions(-)\n> > >\n> > >\n"},{"id":"385116","messageId":"20191029212734.luledidagh7dnx5y@yadavpratyush.com","threadId":"52142","inReplyTo":"56faba848676ffc5441f373bc2105392fc2a3e11.1572289087.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] git-gui: Revert untracked files by deleting them","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-29T21:27:34Z","receivedAt":"2019-10-29T21:27:42Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Jonathan,\n\nThanks for the patch.\n\nWhile git-gui is distributed in the main Git tree, the development \nhappens on a separate repo, and the Git maintainer periodically pulls in \nchanges from that repo. It can be found at [0]. For now, I munged your \npatch to apply on my tree, but please base it on the git-gui repo for \nyour re-rolls or future patches. You can use GitGitGadget to do that \n[1].\n\nNow, on to the patch.\n\nNitpick: Do not use a capital letter after 'git-gui:' in your subject. \nSo the subject should look something like:\n\n  git-gui: revert untracked files by deleting them\n\nOn 28/10/19 06:58PM, Jonathan Gilbert via GitGitGadget wrote:\n> From: Jonathan Gilbert <JonathanG@iQmetrix.com>\n> \n> My development environment sometimes makes automatic changes\n> that I don't want to keep. In some cases, this involves new\n> files being added that I don't want to commit or keep. I have\n> typically had to explicitly delete those files externally to\n> Git Gui, and I want to be able to just select those newly-\n> created untracked files and \"revert\" them into oblivion.\n\nI think the description of your workflow belongs in the cover letter \nmore than here. The commit message should take a more neutral tone. So, \ndescribe the problem in an objective way that not only you, but other \ngit-gui users might face.\n \n> This change updates the revert_helper function to check for\n> untracked files as well as changes, and then any changes to be\n> reverted and untracked files are handled by independent\n> blocks of code. The user is prompted independently for\n> untracked files, since the underlying action is fundamentally\n> different (rm -f). If after deleting untracked files, the\n> directory containing them becomes empty, then the directory is\n> removed as well.\n> \n> This introduces new strings in index.tcl. I have been told that\n> there is a separate process whereby the translations get updated.\n\nI don't think this should be in the commit message. The commit message \ndescribes the change. The process of getting that change integrated \nshould be discussed elsewhere (elsewhere == this list). But yes, there \nis a separate process to update translations (but unfortunately no one \nis actively doing that yet).\n \n> Signed-off-by: Jonathan Gilbert <JonathanG@iQmetrix.com>\n> ---\n>  git-gui/lib/index.tcl | 139 +++++++++++++++++++++++++++++++-----------\n>  1 file changed, 104 insertions(+), 35 deletions(-)\n> \n> diff --git a/git-gui/lib/index.tcl b/git-gui/lib/index.tcl\n> index e07b7a3762..cc1b651d56 100644\n> --- a/git-gui/lib/index.tcl\n> +++ b/git-gui/lib/index.tcl\n> @@ -393,11 +393,24 @@ proc revert_helper {txt paths} {\n>  \n>  \tif {![lock_index begin-update]} return\n>  \n> +\t# If an action is taken that implicitly unlocks the index, this gets cleared. Either way, it is executed at the end of the procedure.\n\nThe convention is to wrap lines at 80 columns wherever possible. Please \nfollow that. You can look at the rest of the code for examples.\n\nYou have other lines too that are too long. The same comment applies to \nall those.\n\n> +\tset epilogue [list]\n> +\tlappend epilogue {unlock_index}\n> +\n> +\tproc already_unlocked {} { upvar epilogue epilogue; set epilogue [lsearch -inline -all -not -exact $epilogue {unlock_index}] }\n\nA procedure defined inside a procedure? Please don't do that. Define it \noutside.\n\nAlso, what is this procedure supposed to do? It is not very clear at \nfirst read.\n\n> +\n>  \tset pathList [list]\n> +\tset untrackedList [list]\n\nNitpick: Ugh! camelCase in a sea of snake_cases. What's even more \nunfortunate is that `pathList` itself is in camelCase, so that's \nprobably the reason you went with camelCase in the first place. Maybe \nre-name `pathList` to `path_list` while we're at it, and then use \nsnake_case everywhere?\n\n>  \tset after {}\n>  \tforeach path $paths {\n>  \t\tswitch -glob -- [lindex $file_states($path) 0] {\n>  \t\tU? {continue}\n> +\t\t?O {\n> +\t\t\tlappend untrackedList $path\n> +\t\t\tif {$path eq $current_diff_path} {\n> +\t\t\t\tset after {reshow_diff;}\n> +\t\t\t}\n> +\t\t}\n>  \t\t?M -\n>  \t\t?T -\n>  \t\t?D {\n> @@ -410,45 +423,101 @@ proc revert_helper {txt paths} {\n>  \t}\n>  \n>  \n> -\t# Split question between singular and plural cases, because\n> -\t# such distinction is needed in some languages. Previously, the\n> -\t# code used \"Revert changes in\" for both, but that can't work\n> -\t# in languages where 'in' must be combined with word from\n> -\t# rest of string (in different way for both cases of course).\n> -\t#\n> -\t# FIXME: Unfortunately, even that isn't enough in some languages\n> -\t# as they have quite complex plural-form rules. Unfortunately,\n> -\t# msgcat doesn't seem to support that kind of string translation.\n> -\t#\n> -\tset n [llength $pathList]\n> -\tif {$n == 0} {\n> -\t\tunlock_index\n> -\t\treturn\n> -\t} elseif {$n == 1} {\n> -\t\tset query [mc \"Revert changes in file %s?\" [short_path [lindex $pathList]]]\n> -\t} else {\n> -\t\tset query [mc \"Revert changes in these %i files?\" $n]\n> -\t}\n> +\tset numPaths [llength $pathList]\n> +\tset numUntracked [llength $untrackedList]\n>  \n> -\tset reply [tk_dialog \\\n> -\t\t.confirm_revert \\\n> -\t\t\"[appname] ([reponame])\" \\\n> -\t\t\"$query\n> +\tif {$numPaths > 0} {\n> +\t\t# Split question between singular and plural cases, because\n> +\t\t# such distinction is needed in some languages. Previously, the\n> +\t\t# code used \"Revert changes in\" for both, but that can't work\n> +\t\t# in languages where 'in' must be combined with word from\n> +\t\t# rest of string (in different way for both cases of course).\n> +\t\t#\n> +\t\t# FIXME: Unfortunately, even that isn't enough in some languages\n> +\t\t# as they have quite complex plural-form rules. Unfortunately,\n> +\t\t# msgcat doesn't seem to support that kind of string translation.\n> +\t\tif {$numPaths == 1} {\n> +\t\t\tset query [mc \"Revert changes in file %s?\" [short_path [lindex $pathList]]]\n> +\t\t} else {\n> +\t\t\tset query [mc \"Revert changes in these %i files?\" $numPaths]\n> +\t\t}\n> +\n> +\t\tset reply [tk_dialog \\\n> +\t\t\t.confirm_revert \\\n> +\t\t\t\"[appname] ([reponame])\" \\\n> +\t\t\t\"$query\n>  \n>  [mc \"Any unstaged changes will be permanently lost by the revert.\"]\" \\\n> -\t\tquestion \\\n> -\t\t1 \\\n> -\t\t[mc \"Do Nothing\"] \\\n> -\t\t[mc \"Revert Changes\"] \\\n> -\t\t]\n> -\tif {$reply == 1} {\n> -\t\tcheckout_index \\\n> -\t\t\t$txt \\\n> -\t\t\t$pathList \\\n> -\t\t\t[concat $after [list ui_ready]]\n> -\t} else {\n> -\t\tunlock_index\n> +\t\t\tquestion \\\n> +\t\t\t1 \\\n> +\t\t\t[mc \"Do Nothing\"] \\\n> +\t\t\t[mc \"Revert Changes\"] \\\n> +\t\t\t]\n> +\n> +\t\tif {$reply == 1} {\n> +\t\t\tcheckout_index \\\n> +\t\t\t\t$txt \\\n> +\t\t\t\t$pathList \\\n> +\t\t\t\t[concat $after [list ui_ready]]\n> +\n> +\t\t\talready_unlocked\n> +\t\t}\n> +\t}\n> +\n> +\tif {$numUntracked > 0} {\n> +\t\t# Split question between singular and plural cases, because\n> +\t\t# such distinction is needed in some languages.\n> +\t\t#\n> +\t\t# FIXME: Unfortunately, even that isn't enough in some languages\n> +\t\t# as they have quite complex plural-form rules. Unfortunately,\n> +\t\t# msgcat doesn't seem to support that kind of string translation.\n> +\t\tif {$numUntracked == 1} {\n> +\t\t\tset query [mc \"Delete untracked file %s?\" [short_path [lindex $untrackedList]]]\n> +\t\t} else {\n> +\t\t\tset query [mc \"Delete these %i untracked files?\" $numUntracked]\n> +\t\t}\n> +\n> +\t\tset reply [tk_dialog \\\n> +\t\t\t.confirm_revert \\\n> +\t\t\t\"[appname] ([reponame])\" \\\n> +\t\t\t\"$query\n> +\n> +[mc \"Files will be permanently deleted.\"]\" \\\n> +\t\t\tquestion \\\n> +\t\t\t1 \\\n> +\t\t\t[mc \"Do Nothing\"] \\\n> +\t\t\t[mc \"Delete Files\"] \\\n> +\t\t\t]\n> +\n> +\t\tif {$reply == 1} {\n> +\t\t\tfile delete -- {*}$untrackedList\n> +\n> +\t\t\tforeach path $untrackedList {\n> +\t\t\t\tset directoryPath [file dirname $path]\n> +\n> +\t\t\t\twhile {$directoryPath != $path} {\n> +\t\t\t\t\tset contents [glob -nocomplain -dir $path *]\n> +\n> +\t\t\t\t\tif {[llength $contents] > 0} { break }\n> +\n> +\t\t\t\t\ttry {\n> +\t\t\t\t\t\tfile delete -- $path\n> +\t\t\t\t\t}\n> +\t\t\t\t\tcatch {\n> +\t\t\t\t\t\t# This is just a best effort, don't annoy the user with failure to remove empty directories.\n> +\t\t\t\t\t\tbreak\n> +\t\t\t\t\t}\n\nThe convention in this project is to just use `catch`, and not try. So \nsomething like:\n\n  catch {file delete -- $path}\n\n> +\n> +\t\t\t\t\tset path $directoryPath\n> +\t\t\t\t\tset directoryPath [file dirname $path]\n\nI read this loop as \"if all the paths in a directory are removed, remove \nthe empty directory as well\". Do I read correctly?\n\nWill there be problems in deleting the directory? What if the user wants \nto keep the directory, and just delete the files? Is that even a valid \nuse-case?\n\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tlappend epilogue {ui_do_rescan}\n\nA rescan is an expensive operation, so we should use it judiciously. Are \nyou sure it is really needed? The \"Revert\" code does not do a rescan but \nstill manages to update the list of \"unstaged files\". How does it manage \nthat? Can the new code do something similar?\n\n> +\t\t}\n>  \t}\n> +\n> +\tforeach epilogueCommand $epilogue { {*}$epilogueCommand }\n\nWhy not use `eval` [2]? Are there any downsides to that compared to your \nway? If not, use `eval`. At least it means better readability if nothing \nelse.\n\nAs far as I see, you use $epilogue for two things: unlocking the index \nand rescanning. Can you move the control flow around that both can be \ndone in the \"normal\" way. That is, they are not a part of a list of \nthings to do at the end, but instead are done when needed. For example, \njust move the call to `unlock_index` at the end instead of putting it in \nepilogue. Can the same be done for `ui_do_rescan` (if you do go with a \nrescan instead of doing it like the existing revert does)?\n\n>  }\n>  \n>  proc do_revert_selection {} {\n\nWhile I appreciate the idea of such a feature, I'm surprised by how \ncomplex the implementation is. I expected something much simpler. The \ncomplexity can probably be managed a bit better by moving the control \nflow around.\n\nI couldn't dive in the code as deep as I wanted to because I don't have \ntoo much time on my hands. But maybe I'll look further by the time your \nre-roll arrives. Thanks.\n\n[0] https://github.com/prati0100/git-gui\n[1] https://github.com/prati0100/git-gui#using-gitgitgadget\n[2] https://www.tcl.tk/man/tcl8.6/TclCmd/eval.htm\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"385118","messageId":"20191029214349.iffk37psv3n4v47y@yadavpratyush.com","threadId":"52142","inReplyTo":"CAPSOpYsA9n6Y1vH1_+vRTCV7Mrvox4L2LpQmpXBnpH8ke0yDhQ@mail.gmail.com","subject":"Re: [PATCH 0/1] Allow the 'revert' option in Git Gui to operate on untracked files, deleting them","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-29T21:43:49Z","receivedAt":"2019-10-29T21:43:59Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 29/10/19 03:25PM, Jonathan Gilbert wrote:\n> That's kind of neat, I wasn't aware of that facet of Git Gui :-) But,\n> it isn't quite the same feature:\n> \n> * It has to be manually set up on each installation.\n> * It invokes an external process, I don't know if it's safe to assume\n> that \"rm\" will work on all platforms (though I just tested it on my\n> Windows installation and it worked).\n> * It doesn't remove directories that it makes empty.\n> * I don't see a way to bind it to a keyboard shortcut. That could just\n> be me not knowing enough about custom tools, though. :-)\n\nYou can't as of now. Harish was in the process of implementing this [0], \nbut I left some comments and he hasn't re-rolled the patch since. So \nunless I find some time to tie it up, this will remain un-implemented. \n\nOf course, if you'd like to see that feature in git-gui, feel free to \npick it up and brush up the changes :). The latest version can be found \nat [1]. But it does not have a proper commit message (apart from the \nother changes I suggested) since Harish did not format the patch \ncorrectly when sending.\n\n> * It only processes the first file selected.\n> * If I select a tracked file, it will still delete it, and the feature\n> I'm looking for is more of a \"return repository to clean state\" type\n> function, like \"revert\" already is but extended to handle files that\n> you can't actually \"git revert\".\n\n[0] https://public-inbox.org/git/CACV9s2MQCP04QASgt0xhi3cSNPSKjwXTufxmZQXAUNvnWD9DSw@mail.gmail.com/ \n[1] https://github.com/prati0100/git-gui/tree/hk/custom-keyboard-shortcuts\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"385120","messageId":"CAPSOpYs7GX6Sg-rYCbhs-s8cTCLkUV=tsfx-JBQhCTP=nSr0_g@mail.gmail.com","threadId":"52142","inReplyTo":"20191029212734.luledidagh7dnx5y@yadavpratyush.com","subject":"Re: [PATCH 1/1] git-gui: Revert untracked files by deleting them","fromName":"Jonathan Gilbert","fromEmail":"logic@deltaq.org","sentAt":"2019-10-29T23:52:25Z","receivedAt":"2019-10-29T23:52:55Z","isPatch":true,"sender":{"key":"logic@deltaq.org","avatar":"https://gravatar.com/avatar/6aa0c0a07ad7988ae46c3c5319d4bbdaa73773b49fb7e668c4bb0aad34b8f67f?d=mp&s=160"},"body":"Thanks for the reply :-)\n\n> While git-gui is distributed in the main Git tree, the development\n> happens on a separate repo, and the Git maintainer periodically pulls in\n> changes from that repo. It can be found at [0]. For now, I munged your\n> patch to apply on my tree, but please base it on the git-gui repo for\n> your re-rolls or future patches. You can use GitGitGadget to do that\n> [1].\n\nAlright :-)\n\n> Now, on to the patch.\n>\n> On 28/10/19 06:58PM, Jonathan Gilbert via GitGitGadget wrote:\n> > From: Jonathan Gilbert <JonathanG@iQmetrix.com>\n> >\n> > My development environment sometimes makes automatic changes\n> > that I don't want to keep. In some cases, this involves new\n> > files being added that I don't want to commit or keep. I have\n> > typically had to explicitly delete those files externally to\n> > Git Gui, and I want to be able to just select those newly-\n> > created untracked files and \"revert\" them into oblivion.\n>\n> I think the description of your workflow belongs in the cover letter\n> more than here. The commit message should take a more neutral tone. So,\n> describe the problem in an objective way that not only you, but other\n> git-gui users might face.\n\nThat's totally fair, I was sort of shooting in the dark since this is\nthe first such patch I have made. I will reword the commit message.\n\n> > +     # If an action is taken that implicitly unlocks the index, this gets cleared. Either way, it is executed at the end of the procedure.\n>\n> The convention is to wrap lines at 80 columns wherever possible. Please\n> follow that. You can look at the rest of the code for examples.\n>\n> You have other lines too that are too long. The same comment applies to\n> all those.\n\nRoger.\n\n> > +     set epilogue [list]\n> > +     lappend epilogue {unlock_index}\n> > +\n> > +     proc already_unlocked {} { upvar epilogue epilogue; set epilogue [lsearch -inline -all -not -exact $epilogue {unlock_index}] }\n>\n> A procedure defined inside a procedure? Please don't do that. Define it\n> outside.\n>\n> Also, what is this procedure supposed to do? It is not very clear at\n> first read.\n\nThe name could probably be improved, but this procedure can't live\noutside of the outer proc because it is lexically tied to it. It takes\nan action on state that is in a local variable. If it's flat-out\ndisallowed to use a proc to abstract this, then every place that wants\nto indicate that the repository is already unlocked and doesn't need\nto be explicitly unlocked in the epilogue will have to repeat the code\ninside the proc.\n\nThis becomes a non-issue if I rework the function so that it doesn't\nend with a dynamic epilogue (see below).\n\n> > +\n> >       set pathList [list]\n> > +     set untrackedList [list]\n>\n> Nitpick: Ugh! camelCase in a sea of snake_cases. What's even more\n> unfortunate is that `pathList` itself is in camelCase, so that's\n> probably the reason you went with camelCase in the first place. Maybe\n> re-name `pathList` to `path_list` while we're at it, and then use\n> snake_case everywhere?\n\nAbsolutely, yeah I was just copying what I already saw there, but I'm\nall in favour of consistency. :-)\n\n> > +     set numPaths [llength $pathList]\n> > +     set numUntracked [llength $untrackedList]\n\nWill fix this too.\n\n> > +                                     try {\n> > +                                             file delete -- $path\n> > +                                     }\n> > +                                     catch {\n> > +                                             # This is just a best effort, don't annoy the user with failure to remove empty directories.\n> > +                                             break\n> > +                                     }\n>\n> The convention in this project is to just use `catch`, and not try. So\n> something like:\n>\n>   catch {file delete -- $path}\n\nI'm not super familiar with TCL, where does the `break` statement fit into this?\n\nI did a Google search and saw that catch returns a value that you can\ninspect, would I write this?:\n```\nif { [catch {file delete -- $path}] } {\n  break\n}\n```\n\n> > +                                     set path $directoryPath\n> > +                                     set directoryPath [file dirname $path]\n>\n> I read this loop as \"if all the paths in a directory are removed, remove\n> the empty directory as well\". Do I read correctly?\n>\n> Will there be problems in deleting the directory? What if the user wants\n> to keep the directory, and just delete the files? Is that even a valid\n> use-case?\n\nWell, Git itself doesn't keep empty directories. As such, I wrote the\ncode with an (undocumented) assumption that if there is a directory\nthat contains only a single untracked file, then the directory was\nprobably created to put the file in it.\n\n> > +                             }\n> > +                     }\n> > +\n> > +                     lappend epilogue {ui_do_rescan}\n>\n> A rescan is an expensive operation, so we should use it judiciously. Are\n> you sure it is really needed? The \"Revert\" code does not do a rescan but\n> still manages to update the list of \"unstaged files\". How does it manage\n> that? Can the new code do something similar?\n\nI'll look into it, but I'm assuming it's happening as part of `checkout_index`.\n\n> > +             }\n> >       }\n> > +\n> > +     foreach epilogueCommand $epilogue { {*}$epilogueCommand }\n>\n> Why not use `eval` [2]? Are there any downsides to that compared to your\n> way? If not, use `eval`. At least it means better readability if nothing\n> else.\n\nI wrote some TCL over a decade ago for Eggdrop bot scripts, and\nhaven't touched it until now, so my ambient knowledge of TCL is quite\nlimited. I'll look into how to use `eval` for this. :-)\n\n> As far as I see, you use $epilogue for two things: unlocking the index\n> and rescanning. Can you move the control flow around that both can be\n> done in the \"normal\" way. That is, they are not a part of a list of\n> things to do at the end, but instead are done when needed. For example,\n> just move the call to `unlock_index` at the end instead of putting it in\n> epilogue. Can the same be done for `ui_do_rescan` (if you do go with a\n> rescan instead of doing it like the existing revert does)?\n\nWell, `unlock_index` will presumably throw an error if\n`checkout_index` gets called, but `checkout_index` only gets called if\nthe scan finds tracked files with changes _and_ the user opts to\nrevert them.\n\nSimilarly, `ui_do_rescan` wants to be done exactly once at the end,\nbut only if the scan finds untracked files _and_ the user opts to\ndelete them.\n\nAn alternative to the epilogue could be two booleans\n`need_unlock_index` that starts out true and `need_rescan` that starts\nout false, and then the function's epilogue, instead of being dynamic,\njust checks the booleans and does the things.\n\n> >  }\n> >\n> >  proc do_revert_selection {} {\n>\n> While I appreciate the idea of such a feature, I'm surprised by how\n> complex the implementation is. I expected something much simpler. The\n> complexity can probably be managed a bit better by moving the control\n> flow around.\n>\n> I couldn't dive in the code as deep as I wanted to because I don't have\n> too much time on my hands. But maybe I'll look further by the time your\n> re-roll arrives. Thanks.\n>\n> [0] https://github.com/prati0100/git-gui\n> [1] https://github.com/prati0100/git-gui#using-gitgitgadget\n> [2] https://www.tcl.tk/man/tcl8.6/TclCmd/eval.htm\n>\n> --\n> Regards,\n> Pratyush Yadav\n"}]}