{"thread":{"id":"9508","subject":"[PATCH] gitk - dont warn when deleting synonym for current head","startedAt":"2007-08-12T21:03:03Z","lastAt":"2007-08-13T13:22:24Z","messageCount":7,"participants":["Mark Levedahl","Paul Mackerras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"50597","messageId":"1186952583887-git-send-email-mdl123@verizon.net","threadId":"9508","inReplyTo":null,"subject":"[PATCH] gitk - dont warn when deleting synonym for current head","fromName":"Mark Levedahl","fromEmail":"mdl123@verizon.net","sentAt":"2007-08-12T21:03:03Z","receivedAt":"2007-08-12T21:03:03Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Selecting \"remove this branch\" on a branch equal to HEAD yielded a\nwarning that this branch is not in any other: it is, it is on HEAD.\nCuriosly, this warning was not triggered for any other deletion and\nso is at best misleading. Get rid of it.\n\nSigned-off-by: Mark Levedahl <mdl123@verizon.net>\n---\n gitk |    6 ------\n 1 files changed, 0 insertions(+), 6 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 769c79a..055cdc7 100755\n--- a/gitk\n+++ b/gitk\n@@ -6171,12 +6171,6 @@ proc rmbranch {} {\n \terror_popup \"Cannot delete the currently checked-out branch\"\n \treturn\n     }\n-    set dheads [descheads $id]\n-    if {$dheads eq $headids($head)} {\n-\t# the stuff on this branch isn't on any other branch\n-\tif {![confirm_popup \"The commits on branch $head aren't on any other\\\n-\t\t\tbranch.\\nReally delete branch $head?\"]} return\n-    }\n     nowbusy rmbranch\n     update\n     if {[catch {exec git branch -D $head} err]} {\n-- \n1.5.3.rc4.80.ge600\n"},{"id":"50606","messageId":"18111.39773.231609.306547@cargo.ozlabs.ibm.com","threadId":"9508","inReplyTo":"1186952583887-git-send-email-mdl123@verizon.net","subject":"Re: [PATCH] gitk - dont warn when deleting synonym for current head","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-08-12T23:44:29Z","receivedAt":"2007-08-12T23:44:29Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Mark Levedahl writes:\n\n> Selecting \"remove this branch\" on a branch equal to HEAD yielded a\n> warning that this branch is not in any other: it is, it is on HEAD.\n\nDo you mean that HEAD was a direct ref to the same commit as the\nbranch you were deleting, or that HEAD pointed to the branch you were\ndeleting?  I.e. what was in .git/HEAD, a commit ID or a branch name?\n\nIf the latter, then I think gitk was correct to warn, since you'll end\nup with .git/HEAD pointing to a non-existent branch, won't you?\n\n> Curiosly, this warning was not triggered for any other deletion and\n> so is at best misleading. Get rid of it.\n\nI'd rather fix it.\n\nPaul.\n"},{"id":"50609","messageId":"46BFAF8E.1000508@verizon.net","threadId":"9508","inReplyTo":"18111.39773.231609.306547@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] gitk - dont warn when deleting synonym for current head","fromName":"Mark Levedahl","fromEmail":"mdl123@verizon.net","sentAt":"2007-08-13T01:10:38Z","receivedAt":"2007-08-13T01:10:38Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Paul Mackerras wrote:\n> Mark Levedahl writes:\n>\n>   \n> Do you mean that HEAD was a direct ref to the same commit as the\n> branch you were deleting, or that HEAD pointed to the branch you were\n> deleting?  I.e. what was in .git/HEAD, a commit ID or a branch name?\n>\n> If the latter, then I think gitk was correct to warn, since you'll end\n> up with .git/HEAD pointing to a non-existent branch, won't you?\n>   \ncurrent checked out branch = master\nHEAD contains ref: refs/heads/master\nfoo points to the same commit as master\nI attempted to delete foo.\n\nNote: the context menu does not allow deleting the currently checked out \nbranch, so the referenced check is irrelevant to that.\n\nMark\n"},{"id":"50610","messageId":"46BFB908.7050000@gmail.com","threadId":"9508","inReplyTo":"46BFAF8E.1000508@verizon.net","subject":"Re: [PATCH] gitk - dont warn when deleting synonym for current head","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-08-13T01:51:04Z","receivedAt":"2007-08-13T01:51:04Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Mark Levedahl wrote:\n> current checked out branch = master\n> HEAD contains ref: refs/heads/master\n> foo points to the same commit as master\n> I attempted to delete foo.\n>\n> Note: the context menu does not allow deleting the currently checked \n> out branch, so the referenced check is irrelevant to that.\n>\nBy the way, I'm not sure how gitk can usefully do the implied check \nwithout loading the full repository DAG. The check requires finding that \nthe referenced commit is not reachable by anything in .git/refs/* except \nthe branch head in question, yet in general gitk has not explored all of \nthose refs, only those explicitly given on the command line (or HEAD by \ndefault). So, unless \"delete branch\" is going to require a variant of \n\"git-rev-list --all\", I think the choices are either a) do what the user \nrequested, or b) offer a \"Are you sure you want to do this?\" dialog \nfollowed by doing what the user asked.\n\ngit-branch -d is similarly challenged: it complains unless the branch to \ndelete is reachable from the current HEAD, and requires -D to delete \nanything else. It does not check to see if the commit is otherwise \nreachable from .git/refs/*.\n\nMark\n"},{"id":"50613","messageId":"18111.54668.327095.685366@cargo.ozlabs.ibm.com","threadId":"9508","inReplyTo":"46BFB908.7050000@gmail.com","subject":"Re: [PATCH] gitk - dont warn when deleting synonym for current head","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-08-13T03:52:44Z","receivedAt":"2007-08-13T03:52:44Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Mark Levedahl writes:\n\n> By the way, I'm not sure how gitk can usefully do the implied check \n> without loading the full repository DAG.\n\nIt does load the full repository DAG - that's how it gets the\ninformation for the Precedes:, Follows: and Branch(es): fields in the\ncommit display.  It's true that that can be turned off, though, and in\nthat case an \"are you sure\" dialog would be appropriate.\n\nPaul.\n"},{"id":"50633","messageId":"30e4a070708130604g5b848b2fm4b42e145384c0ba4@mail.gmail.com","threadId":"9508","inReplyTo":"18111.54668.327095.685366@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] gitk - dont warn when deleting synonym for current head","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-08-13T13:04:52Z","receivedAt":"2007-08-13T13:04:52Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"On 8/12/07, Paul Mackerras <paulus@samba.org> wrote:\n> Mark Levedahl writes:\n> It does load the full repository DAG - that's how it gets the\n> information for the Precedes:, Follows: and Branch(es): fields in the\n> commit display.  It's true that that can be turned off, though, and in\n> that case an \"are you sure\" dialog would be appropriate.\n>\n> Paul.\n>\nIf I do bare gitk, gitk loads the DAG only for HEAD, it does not read\nwhat else is there. For instance,\n"},{"id":"50634","messageId":"30e4a070708130622l7558230cpb870378aeba537fb@mail.gmail.com","threadId":"9508","inReplyTo":"30e4a070708130604g5b848b2fm4b42e145384c0ba4@mail.gmail.com","subject":"Re: [PATCH] gitk - dont warn when deleting synonym for current head","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-08-13T13:22:24Z","receivedAt":"2007-08-13T13:22:24Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"On 8/13/07, Mark Levedahl <mlevedahl@gmail.com> wrote:\n> On 8/12/07, Paul Mackerras <paulus@samba.org> wrote:\n> > It does load the full repository DAG - that's how it gets the\n> > information for the Precedes:, Follows: and Branch(es): fields in the\n> > commit display.  It's true that that can be turned off, though, and in\n> > that case an \"are you sure\" dialog would be appropriate.\n> >\n> > Paul.\n>\n\nDouble groan - I hit the wrong key (send rather than abort) with a\npartially written incorrect message. Drat.\n\nI was fooled as the \"Precedes:\" entry is blank if I do, for instance:\n   git checkout -b foo master~1\n   gitk\neven though HEAD=foo which precedes master. However, master shows up\nin the list of branches containing the commit. So yes, I see that gitk\nhas the full DAG available and could therefore generate a useful\nwarning when deleting a branch would leave dangling commits.\n\nMark\n"}]}