threads / patch / 5691

patchgitk: Fix nextfile() and more

Subject: [PATCH] gitk: Fix nextfile() and more

## tl;dr

7 messages between Sep 25, 2006 and Sep 27, 2006. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

OGAWA Hirofumi· Sep 25, 2006, 13:46 UTC · lore
Hi,

The current nextfile() jumps to last hunk, but I think this is not intention, probably, it's forgetting to add "break;". Right? And this patch also adds prevfile(), it jumps to previous hunk.

The following part is just my favorite the key-binds, it doesn't matter.
+    bind . <Control-p> "$ctext yview scroll -1 units"
+    bind . <Control-n> "$ctext yview scroll 1 units"
+    bind . <Alt-v> "$ctext yview scroll -1 pages"
+    bind . <Control-v> "$ctext yview scroll 1 pages"
+    bindkey P prevfile
+    bindkey N nextfile
What do you think of this?
Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>


Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 gitk |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff -puN gitk~gitk-key-emacs gitk
--- git/gitk~gitk-key-emacs	2006-09-25 22:31:51.000000000 +0900
+++ git-hirofumi/gitk	2006-09-25 22:31:51.000000000 +0900
@@ -672,6 +672,10 @@ proc makewindow {} {
     bind . <Control-Key-Down> "allcanvs yview scroll 1 units"
     bind . <Control-Key-Prior> "allcanvs yview scroll -1 pages"
     bind . <Control-Key-Next> "allcanvs yview scroll 1 pages"
+    bind . <Control-p> "$ctext yview scroll -1 units"
+    bind . <Control-n> "$ctext yview scroll 1 units"
+    bind . <Alt-v> "$ctext yview scroll -1 pages"
+    bind . <Control-v> "$ctext yview scroll 1 pages"
     bindkey <Key-Delete> "$ctext yview scroll -1 pages"
     bindkey <Key-BackSpace> "$ctext yview scroll -1 pages"
     bindkey <Key-space> "$ctext yview scroll 1 pages"
@@ -690,6 +694,8 @@ proc makewindow {} {
     bindkey <Key-Return> {findnext 0}
     bindkey ? findprev
     bindkey f nextfile
+    bindkey P prevfile
+    bindkey N nextfile
     bind . <Control-q> doquit
     bind . <Control-f> dofind
     bind . <Control-g> {findnext 0}
@@ -4440,12 +4446,26 @@ proc getblobdiffline {bdf ids} {
     }
 }
 
+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
+	    break
+	}
+	set prev $loc
+    }
+}
+
 proc nextfile {} {
     global difffilestart ctext
     set here [$ctext index @0,0]
     foreach loc $difffilestart {
 	if {[$ctext compare $loc > $here]} {
 	    $ctext yview $loc
+	    break
 	}
     }
 }
_
Junio C Hamano· Sep 26, 2006, 05:24 UTC · re: OGAWA Hirofumi · lore

Re: [PATCH] gitk: Fix nextfile() and more

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:
Show 14 quoted lines
> The current nextfile() jumps to last hunk, but I think this is not
> intention, probably, it's forgetting to add "break;". Right?  And this
> patch also adds prevfile(), it jumps to previous hunk.
>
> The following part is just my favorite the key-binds, it doesn't matter.
>
> +    bind . <Control-p> "$ctext yview scroll -1 units"
> +    bind . <Control-n> "$ctext yview scroll 1 units"
> +    bind . <Alt-v> "$ctext yview scroll -1 pages"
> +    bind . <Control-v> "$ctext yview scroll 1 pages"
> +    bindkey P prevfile
> +    bindkey N nextfile
>
> What do you think of this?
. As a patch e-mail, the initial Hi and trailing Thanks add
  additional work for the person who applies patch.
. ACK on missing "break" and addition of prevfile for symmetry.
. I do not have preference on the extra bindings.  I do not
  speak for Paul, but I suspect he would have liked them in a
  separate patch.
OGAWA Hirofumi· Sep 26, 2006, 15:19 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitk: Fix nextfile() and more

Junio C Hamano <junkio@cox.net> writes:
Show 19 quoted lines
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:
>
>> The current nextfile() jumps to last hunk, but I think this is not
>> intention, probably, it's forgetting to add "break;". Right?  And this
>> patch also adds prevfile(), it jumps to previous hunk.
>>
>> The following part is just my favorite the key-binds, it doesn't matter.
>>
>> +    bind . <Control-p> "$ctext yview scroll -1 units"
>> +    bind . <Control-n> "$ctext yview scroll 1 units"
>> +    bind . <Alt-v> "$ctext yview scroll -1 pages"
>> +    bind . <Control-v> "$ctext yview scroll 1 pages"
>> +    bindkey P prevfile
>> +    bindkey N nextfile
>>
>> What do you think of this?
>
> . As a patch e-mail, the initial Hi and trailing Thanks add
>   additional work for the person who applies patch.
Ok.
> . I do not have preference on the extra bindings.  I do not
>   speak for Paul, but I suspect he would have liked them in a
>   separate patch.
Ok. I'll send patches.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
OGAWA Hirofumi· Sep 26, 2006, 15:22 UTC · re: Junio C Hamano · lore

[PATCH] gitk: Fix nextfile() and add prevfile()

The current nextfile() jumps to last hunk, but I think this is not intention, probably, it's forgetting to add "break;". And this patch also adds prevfile(), it jumps to previous hunk.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
 gitk |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff -puN gitk~gitk-nextfile-fix gitk --- git/gitk~gitk-nextfile-fix 2006-09-27 00:16:11.000000000 +0900 +++ git-hirofumi/gitk 2006-09-27 00:16:48.000000000 +0900

Show changes to diff +14 −0
@@ -4440,12 +4440,26 @@ proc getblobdiffline {bdf ids} {
     }
 }
 
+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
+	    break
+	}
+	set prev $loc
+    }
+}
+
 proc nextfile {} {
     global difffilestart ctext
     set here [$ctext index @0,0]
     foreach loc $difffilestart {
 	if {[$ctext compare $loc > $here]} {
 	    $ctext yview $loc
+	    break
 	}
     }
 }
_
OGAWA Hirofumi· Sep 26, 2006, 15:27 UTC · re: OGAWA Hirofumi · lore

[PATCH] gitk: add some key-bind

This add initial user of prevfile(), and additional some key-binds for emacs users.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
 gitk |    6 ++++++
 1 file changed, 6 insertions(+)

diff -puN gitk~gitk-key-emacs gitk --- git/gitk~gitk-key-emacs 2006-09-27 00:17:31.000000000 +0900 +++ git-hirofumi/gitk 2006-09-27 00:17:31.000000000 +0900

Show changes to diff +6 −0
@@ -672,6 +672,10 @@ proc makewindow {} {
     bind . <Control-Key-Down> "allcanvs yview scroll 1 units"
     bind . <Control-Key-Prior> "allcanvs yview scroll -1 pages"
     bind . <Control-Key-Next> "allcanvs yview scroll 1 pages"
+    bind . <Control-p> "$ctext yview scroll -1 units"
+    bind . <Control-n> "$ctext yview scroll 1 units"
+    bind . <Alt-v> "$ctext yview scroll -1 pages"
+    bind . <Control-v> "$ctext yview scroll 1 pages"
     bindkey <Key-Delete> "$ctext yview scroll -1 pages"
     bindkey <Key-BackSpace> "$ctext yview scroll -1 pages"
     bindkey <Key-space> "$ctext yview scroll 1 pages"
@@ -690,6 +694,8 @@ proc makewindow {} {
     bindkey <Key-Return> {findnext 0}
     bindkey ? findprev
     bindkey f nextfile
+    bindkey P prevfile
+    bindkey N nextfile
     bind . <Control-q> doquit
     bind . <Control-f> dofind
     bind . <Control-g> {findnext 0}
_
Paul Mackerras· Sep 27, 2006, 00:15 UTC · re: OGAWA Hirofumi · lore

Re: [PATCH] gitk: Fix nextfile() and add prevfile()

OGAWA Hirofumi writes:
> The current nextfile() jumps to last hunk, but I think this is not
> intention, probably, it's forgetting to add "break;". And this
> patch also adds prevfile(), it jumps to previous hunk.

I think your prevfile isn't quite right - I don't think it will do the right thing if $loc is past the last entry in $difffilestart. Don't you want the "$ctext yview $prev" after the loop?

Paul.
OGAWA Hirofumi· Sep 27, 2006, 03:32 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk: Fix nextfile() and add prevfile()

Paul Mackerras <paulus@samba.org> writes:
Show 11 quoted lines
> OGAWA Hirofumi writes:
>
>> The current nextfile() jumps to last hunk, but I think this is not
>> intention, probably, it's forgetting to add "break;". And this
>> patch also adds prevfile(), it jumps to previous hunk.
>
> I think your prevfile isn't quite right - I don't think it will do the
> right thing if $loc is past the last entry in $difffilestart.  Don't
> you want the "$ctext yview $prev" after the loop?
>
> Paul.
Sure, it's obviously useful in the case of big hunk. Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>


The current nextfile() jumps to last hunk, but I think this is not
intention, probably, it's forgetting to add "break;". And this
patch also adds prevfile(), it jumps to previous hunk.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 gitk |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff -puN gitk~gitk-nextfile-fix gitk
--- git/gitk~gitk-nextfile-fix	2006-09-27 12:14:07.000000000 +0900
+++ git-hirofumi/gitk	2006-09-27 12:16:23.000000000 +0900
@@ -4440,12 +4440,27 @@ proc getblobdiffline {bdf ids} {
     }
 }
 
+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
+	    return
+	}
+	set prev $loc
+    }
+    $ctext yview $prev
+}
+
 proc nextfile {} {
     global difffilestart ctext
     set here [$ctext index @0,0]
     foreach loc $difffilestart {
 	if {[$ctext compare $loc > $here]} {
 	    $ctext yview $loc
+	    return
 	}
     }
 }
_

← back to recent threads