threads / patch / 9508

patchgitk - dont warn when deleting synonym for current head

Subject: [PATCH] gitk - dont warn when deleting synonym for current head

## tl;dr

7 messages between Aug 12, 2007 and Aug 13, 2007. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Mark Levedahl· Aug 12, 2007, 21:03 UTC · lore

Selecting "remove this branch" on a branch equal to HEAD yielded a warning that this branch is not in any other: it is, it is on HEAD. Curiosly, this warning was not triggered for any other deletion and so is at best misleading. Get rid of it.

Signed-off-by: Mark Levedahl <mdl123@verizon.net>
---
 gitk |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

Show changes to gitk +0 −6

diff --git a/gitk b/gitk
index 769c79a..055cdc7 100755
--- a/gitk
+++ b/gitk
@@ -6171,12 +6171,6 @@ proc rmbranch {} {
 	error_popup "Cannot delete the currently checked-out branch"
 	return
     }
-    set dheads [descheads $id]
-    if {$dheads eq $headids($head)} {
-	# the stuff on this branch isn't on any other branch
-	if {![confirm_popup "The commits on branch $head aren't on any other\
-			branch.\nReally delete branch $head?"]} return
-    }
     nowbusy rmbranch
     update
     if {[catch {exec git branch -D $head} err]} {

-- 
1.5.3.rc4.80.ge600

Paul Mackerras· Aug 12, 2007, 23:44 UTC · re: Mark Levedahl · lore

Re: [PATCH] gitk - dont warn when deleting synonym for current head

Mark Levedahl writes:

> Selecting "remove this branch" on a branch equal to HEAD yielded a
> warning that this branch is not in any other: it is, it is on HEAD.

Do you mean that HEAD was a direct ref to the same commit as the branch you were deleting, or that HEAD pointed to the branch you were deleting? I.e. what was in .git/HEAD, a commit ID or a branch name?

If the latter, then I think gitk was correct to warn, since you'll end up with .git/HEAD pointing to a non-existent branch, won't you?

> Curiosly, this warning was not triggered for any other deletion and
> so is at best misleading. Get rid of it.

I'd rather fix it.

Paul.

Mark Levedahl· Aug 13, 2007, 01:10 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk - dont warn when deleting synonym for current head

Paul Mackerras wrote:

Show 10 quoted lines

> Mark Levedahl writes:
>
>   
> Do you mean that HEAD was a direct ref to the same commit as the
> branch you were deleting, or that HEAD pointed to the branch you were
> deleting?  I.e. what was in .git/HEAD, a commit ID or a branch name?
>
> If the latter, then I think gitk was correct to warn, since you'll end
> up with .git/HEAD pointing to a non-existent branch, won't you?
>

current checked out branch = master HEAD contains ref: refs/heads/master foo points to the same commit as master I attempted to delete foo.

Note: the context menu does not allow deleting the currently checked out 
branch, so the referenced check is irrelevant to that.

Mark

Mark Levedahl· Aug 13, 2007, 01:51 UTC · re: Mark Levedahl · lore

Re: [PATCH] gitk - dont warn when deleting synonym for current head

Mark Levedahl wrote:

Show 8 quoted lines

> current checked out branch = master
> HEAD contains ref: refs/heads/master
> foo points to the same commit as master
> I attempted to delete foo.
>
> Note: the context menu does not allow deleting the currently checked 
> out branch, so the referenced check is irrelevant to that.
>

By the way, I'm not sure how gitk can usefully do the implied check without loading the full repository DAG. The check requires finding that the referenced commit is not reachable by anything in .git/refs/* except the branch head in question, yet in general gitk has not explored all of those refs, only those explicitly given on the command line (or HEAD by default). So, unless "delete branch" is going to require a variant of "git-rev-list --all", I think the choices are either a) do what the user requested, or b) offer a "Are you sure you want to do this?" dialog followed by doing what the user asked.

git-branch -d is similarly challenged: it complains unless the branch to delete is reachable from the current HEAD, and requires -D to delete anything else. It does not check to see if the commit is otherwise reachable from .git/refs/*.

Mark

Paul Mackerras· Aug 13, 2007, 03:52 UTC · re: Mark Levedahl · lore

Re: [PATCH] gitk - dont warn when deleting synonym for current head

Mark Levedahl writes:

> By the way, I'm not sure how gitk can usefully do the implied check 
> without loading the full repository DAG.

It does load the full repository DAG - that's how it gets the information for the Precedes:, Follows: and Branch(es): fields in the commit display. It's true that that can be turned off, though, and in that case an "are you sure" dialog would be appropriate.

Paul.

Mark Levedahl· Aug 13, 2007, 13:04 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk - dont warn when deleting synonym for current head

On 8/12/07, Paul Mackerras <paulus@samba.org> wrote:

Show 8 quoted lines

> Mark Levedahl writes:
> It does load the full repository DAG - that's how it gets the
> information for the Precedes:, Follows: and Branch(es): fields in the
> commit display.  It's true that that can be turned off, though, and in
> that case an "are you sure" dialog would be appropriate.
>
> Paul.
>

If I do bare gitk, gitk loads the DAG only for HEAD, it does not read what else is there. For instance,

Mark Levedahl· Aug 13, 2007, 13:22 UTC · re: Mark Levedahl · lore

Re: [PATCH] gitk - dont warn when deleting synonym for current head

On 8/13/07, Mark Levedahl <mlevedahl@gmail.com> wrote:

Show 8 quoted lines

> On 8/12/07, Paul Mackerras <paulus@samba.org> wrote:
> > It does load the full repository DAG - that's how it gets the
> > information for the Precedes:, Follows: and Branch(es): fields in the
> > commit display.  It's true that that can be turned off, though, and in
> > that case an "are you sure" dialog would be appropriate.
> >
> > Paul.
>

Double groan - I hit the wrong key (send rather than abort) with a partially written incorrect message. Drat.

I was fooled as the "Precedes:" entry is blank if I do, for instance:
   git checkout -b foo master~1
   gitk
even though HEAD=foo which precedes master. However, master shows up
in the list of branches containing the commit. So yes, I see that gitk
has the full DAG available and could therefore generate a useful
warning when deleting a branch would leave dangling commits.

Mark