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

Subsystems: the rest

DORMANTno replies

7 messages, 3 authors, 2016-06-15 · open the first message on its own page

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

From: Mark Levedahl <hidden>
Date: 2016-06-15 22:43:29

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 <redacted>
---
 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

From: Paul Mackerras <hidden>
Date: 2016-06-15 22:43:29

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

From: Mark Levedahl <hidden>
Date: 2016-06-15 22:43:29

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

From: Mark Levedahl <hidden>
Date: 2016-06-15 22:43:29

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

From: Paul Mackerras <hidden>
Date: 2016-06-15 22:43:29

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

From: Mark Levedahl <hidden>
Date: 2016-06-15 22:43:29

On 8/12/07, Paul Mackerras [off-list ref] 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

From: Mark Levedahl <hidden>
Date: 2016-06-15 22:43:29

On 8/13/07, Mark Levedahl [off-list ref] wrote:
On 8/12/07, Paul Mackerras [off-list ref] wrote:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help