From: Linus Torvalds <torvalds@linux-foundation.org>
To: Paul Mackerras <paulus@samba.org>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: gitk: Turn short SHA1 names into links too
Date: Thu, 25 Sep 2008 17:11:42 -0700 (PDT) [thread overview]
Message-ID: <alpine.LFD.1.10.0809251657080.3265@nehalem.linux-foundation.org> (raw)
Ok, so I'm a newbie when it comes to tcl/tk, but I can do copy-paste and
make things work.
And the thing I wanted to work was to have the abbreviated SHA1's that
have started to get more common in the kernel commit logs work as links in
gitk too, just the way a full 40-character SHA1 link works.
This patch does seem to work, but it's also buggy in exactly the same ways
the regular 40-character links are buggy, and while I find those bugs
very irritating _too_, I can't cut-and-paste myself to a solution for
that.
The pre-existing bugs that this shares with the long links are:
- since gitk started doing incremental showing of the graph, the whole
"check if it exists" doesn't work right if the target hasn't been
loaded yet. And when it _does_ end up being loaded one second later,
nothing re-does the scanning.
This is just a small irritation, but it's quite common when the first
commit that is displayed has a link. You can fix it by moving to the
next commit and moving right back (cursor-down + cursor-up), which will
re-display the commit and now find the link that wasn't available
initially, but it's still irritating.
I think gitk could re-display the commit it is on when the whole list
of commits has been parsed, and at least then show the links it missed
initially after a few seconds.
- slightly related to the above: when we _do_ find a link, we create it
to be a link to line so-and-so, but since we now don't just
incrementally parse the commits that come in, but gitk _also_ actually
reflows the commits to be in topological order, the link we just
created may actually no longer point to the right line by the time the
link is then clicked on, so clicking on a link can actually take you to
the wrong commit!
Again, re-displaying the current commit after we have gotten and
parsed all commits will fix it, but this is a more fundamental problem:
if we redisplay at the end, there is still a window when the link may
simply be wrong, because we've redone the topo sort, but we haven't
seen _all_ commits yet.
But again, you can work around it by going back and retrying, and at
some time it will stabilize. But this one is _really_ irritating when
it triggers, because it can make you look at the wrong commit without
necessarily realizing it was wrong!
I suspect that the correct fix is to always do the link, whether we
actually see it or not, and not make it point to a line number, but simply
keep it as a SHA1, and then do the equivalent of "gotocommit" when
clicking it. But I don't know how links workin tcl/tk, so I'm not going to
be able to do that.
In the meantime, this patch introduces no new bugs, and the workarounds
are the same for abbreviated SHA1's as they are for the full ones.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
I'm sure it could have been done better. In particular, I think the
"short->long" translation could/should probably be a function of its own.
But I'm so uncomfortable with wish programming that I'm not starting to
write any new functions..
Comments? Paul?
Linus
gitk-git/gitk | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 2eaa2ae..f79643a 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -5759,7 +5759,7 @@ proc appendwithlinks {text tags} {
set start [$ctext index "end - 1c"]
$ctext insert end $text $tags
- set links [regexp -indices -all -inline {[0-9a-f]{40}} $text]
+ set links [regexp -indices -all -inline {[0-9a-f]{6,40}} $text]
foreach l $links {
set s [lindex $l 0]
set e [lindex $l 1]
@@ -5773,7 +5773,18 @@ proc appendwithlinks {text tags} {
}
proc setlink {id lk} {
- global curview ctext pendinglinks commitinterest
+ global curview ctext pendinglinks commitinterest varcid
+
+ # Turn a short ID into a full one
+ if {[regexp {^[0-9a-f]{4,39}$} $id]} {
+ set matches [array names varcid "$curview,$id*"]
+ if {$matches ne {}} {
+ if {[llength $matches] > 1} {
+ return
+ }
+ set id [lindex [split [lindex $matches 0] ","] 1]
+ }
+ }
if {[commitinview $id $curview]} {
$ctext tag conf $lk -foreground blue -underline 1
next reply other threads:[~2008-09-26 0:12 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-26 0:11 Linus Torvalds [this message]
2008-09-26 0:37 ` gitk: Turn short SHA1 names into links too Linus Torvalds
2008-09-26 6:32 ` Wincent Colaiuta
2008-09-26 7:26 ` Andreas Ericsson
2008-09-26 7:29 ` Wincent Colaiuta
2008-09-26 8:21 ` Mikael Magnusson
2008-09-26 12:15 ` Brad King
2008-09-26 10:41 ` Marco Costalba
2008-09-27 3:18 ` Paul Mackerras
2008-09-27 3:16 ` Paul Mackerras
2008-10-20 23:20 ` Paul Mackerras
2008-10-21 19:09 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LFD.1.10.0809251657080.3265@nehalem.linux-foundation.org \
--to=torvalds@linux-foundation.org \
--cc=git@vger.kernel.org \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).