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

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

[Mark Levedahl]

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(-)

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

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

[Paul Mackerras]

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.

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

[Mark Levedahl]

Paul Mackerras wrote:
> 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

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

[Mark Levedahl]

Mark Levedahl wrote:
> 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

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

[Paul Mackerras]

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.

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

[Mark Levedahl]

On 8/12/07, Paul Mackerras <paulus@samba.org> wrote:
> 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,

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

[Mark Levedahl]

On 8/13/07, Mark Levedahl <mlevedahl@gmail.com> wrote:
> 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