threads / patch / 12720

patchgitk: synchronize highlighting in file view for 'f'&'b' commands

Subject: [PATCH] gitk: synchronize highlighting in file view for 'f'&'b' commands

## tl;dr

2 messages between Mar 17, 2008 and Mar 17, 2008. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Eric Raible· Mar 17, 2008, 02:50 UTC · lore
>From 03603a207b2caad8b52dd4a24860e387d418f800 Mon Sep 17 00:00:00 2001
From: Eric Raible <raible+git@gmail.com>
Date: Sun, 16 Mar 2008 19:33:52 -0700
Subject: [PATCH] gitk: synchronize highlighting in file view for
'f'&'b' commands

Previously, 'b', backspace, and delete all did the same thing. This changes 'b' to perform the inverse of 'f'. And both of them now highlight the filename of the currently diff.

This makes it easier to review the diffs associated with a particular commit using only f,b, and space.

Signed-off-by: Eric Raible <raible+git@gmail.com>
---
 gitk-git/gitk |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)
Show changes to gitk-git/gitk +20 −3
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 84ab02e..e9ca6f7 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -1016,7 +1016,7 @@ proc makewindow {} {
     bindkey k "selnextline 1"
     bindkey j "goback"
     bindkey l "goforw"
-    bindkey b "$ctext yview scroll -1 pages"
+    bindkey b prevfile
     bindkey d "$ctext yview scroll 18 units"
     bindkey u "$ctext yview scroll -18 units"
     bindkey / {dofind 1 1}
@@ -5478,13 +5478,30 @@ proc changediffdisp {} {
     $ctext tag conf d1 -elide [lindex $diffelide 1]
 }

+proc highlightfile {loc} {
+    global ctext
+    $ctext yview $loc
+
+    global cmitmode ctext cflist cflist_top
+    if {$cmitmode eq "tree"} return
+
+    $cflist tag remove highlight $cflist_top.0 "$cflist_top.0 lineend"
+
+    set file [regsub -- "-* (.*?) -*" [$ctext get $loc "$loc lineend"] "\\1"]
+    set cline [$cflist search -regexp [subst {^$file$}] 0.0]
+
+    $cflist tag add highlight $cline "$cline lineend"
+    $cflist see $cline
+    set cflist_top [lindex [split $cline .] 0]
+}
+
 proc prevfile {} {
     global difffilestart ctext
     set prev [lindex $difffilestart 0]
     set here [$ctext index @0,0]
     foreach loc $difffilestart {
 	if {[$ctext compare $loc >= $here]} {
-	    $ctext yview $prev
+	    highlightfile $prev
 	    return
 	}
 	set prev $loc
@@ -5497,7 +5514,7 @@ proc nextfile {} {
     set here [$ctext index @0,0]
     foreach loc $difffilestart {
 	if {[$ctext compare $loc > $here]} {
-	    $ctext yview $loc
+	    highlightfile $loc
 	    return
 	}
     }

-- 
1.5.4.2.1161.g1a6f0
Junio C Hamano· Mar 17, 2008, 07:42 UTC · re: Eric Raible · lore

Re: [PATCH] gitk: synchronize highlighting in file view for 'f'&'b' commands

"Eric Raible" <raible@gmail.com> writes:
Show 5 quoted lines
> From 03603a207b2caad8b52dd4a24860e387d418f800 Mon Sep 17 00:00:00 2001
> From: Eric Raible <raible+git@gmail.com>
> Date: Sun, 16 Mar 2008 19:33:52 -0700
> Subject: [PATCH] gitk: synchronize highlighting in file view for
> 'f'&'b' commands

Please do not do this. The first line is not part of anything but is only a mail message boundary in mbox format. Reproducing From: is fine if the patch author is different from the person who is sending the patch, but I do not think it is necessary in this case. Date and Subject should also go, as taking them from the e-mail header is just as good.

And please do not send gitk patches to me. It should come through Paul Mackerras <paulus@samba.org>, so if your patch is obviously correct for application, then To: him with Cc: list (and I do not mind being on the Cc:), otherwise if the patch is for review and discussion, please send it addressed To: the list (and again I do not mind being on the Cc:).

Show 6 quoted lines
> Previously, 'b', backspace, and delete all did the same thing.
> This changes 'b' to perform the inverse of 'f'.  And both of
> them now highlight the filename of the currently diff.
>
> This makes it easier to review the diffs associated with
> a particular commit using only f,b, and space.

As to the change itself, I think it makes sense to have a binding available for prevfile (previously there was none), so I'd be supportive of that keybinding change.

Show 16 quoted lines
> +proc highlightfile {loc} {
> +    global ctext
> +    $ctext yview $loc
> +
> +    global cmitmode ctext cflist cflist_top
> +    if {$cmitmode eq "tree"} return
> +
> +    $cflist tag remove highlight $cflist_top.0 "$cflist_top.0 lineend"
> +
> +    set file [regsub -- "-* (.*?) -*" [$ctext get $loc "$loc lineend"] "\\1"]
> +    set cline [$cflist search -regexp [subst {^$file$}] 0.0]
> +
> +    $cflist tag add highlight $cline "$cline lineend"
> +    $cflist see $cline
> +    set cflist_top [lindex [split $cline .] 0]
> +}

I however think you also should describe why you had to change "$ctext yview" in both nextfile and prevfile to a more complex "highlightfile" in your commit message. It is not very obvious how it is an improvement, and typing 'b' in tree view (as opposed to patch view) seems to trigger

	Error: bad text index""
with this patch.

← back to recent threads