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

7 messages from 2006-09-25 to 2006-09-27. Participants: OGAWA Hirofumi, Junio C Hamano, Paul Mackerras.
Thread: https://gitlist.dev/t/5691

## OGAWA Hirofumi, 2006-09-25 13:46

Subject: [PATCH] gitk: Fix nextfile() and more
Message-ID: <873bag12k3.fsf@duaron.myhome.or.jp>
URL: https://gitlist.dev/e/873bag12k3.fsf%40duaron.myhome.or.jp

```
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, 2006-09-26 05:24

Subject: Re: [PATCH] gitk: Fix nextfile() and more
Message-ID: <7vpsdjryj2.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7vpsdjryj2.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <873bag12k3.fsf@duaron.myhome.or.jp>

```
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.

. 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, 2006-09-26 15:19

Subject: Re: [PATCH] gitk: Fix nextfile() and more
Message-ID: <877izq3bbf.fsf@duaron.myhome.or.jp>
URL: https://gitlist.dev/e/877izq3bbf.fsf%40duaron.myhome.or.jp
In-Reply-To: <7vpsdjryj2.fsf@assigned-by-dhcp.cox.net>

```
Junio C Hamano <junkio@cox.net> writes:

> 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, 2006-09-26 15:22

Subject: [PATCH] gitk: Fix nextfile() and add prevfile()
Message-ID: <873bae3b5x.fsf_-_@duaron.myhome.or.jp>
URL: https://gitlist.dev/e/873bae3b5x.fsf_-_%40duaron.myhome.or.jp
In-Reply-To: <7vpsdjryj2.fsf@assigned-by-dhcp.cox.net>

```
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
@@ -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, 2006-09-26 15:27

Subject: [PATCH] gitk: add some key-bind
Message-ID: <87y7s61wdx.fsf_-_@duaron.myhome.or.jp>
URL: https://gitlist.dev/e/87y7s61wdx.fsf_-_%40duaron.myhome.or.jp
In-Reply-To: <873bae3b5x.fsf_-_@duaron.myhome.or.jp>

```
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
@@ -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, 2006-09-27 00:15

Subject: Re: [PATCH] gitk: Fix nextfile() and add prevfile()
Message-ID: <17689.49841.913304.878599@cargo.ozlabs.ibm.com>
URL: https://gitlist.dev/e/17689.49841.913304.878599%40cargo.ozlabs.ibm.com
In-Reply-To: <873bae3b5x.fsf_-_@duaron.myhome.or.jp>

```
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, 2006-09-27 03:32

Subject: Re: [PATCH] gitk: Fix nextfile() and add prevfile()
Message-ID: <87fyeernlo.fsf@duaron.myhome.or.jp>
URL: https://gitlist.dev/e/87fyeernlo.fsf%40duaron.myhome.or.jp
In-Reply-To: <17689.49841.913304.878599@cargo.ozlabs.ibm.com>

```
Paul Mackerras <paulus@samba.org> writes:

> 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
 	}
     }
 }
_

```