[PATCH] gitk: Fix nextfile() and more

[PATCH] gitk: Fix nextfile() and more

[OGAWA Hirofumi]

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

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

[Junio C Hamano]

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.

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

[OGAWA Hirofumi]

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>

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

[OGAWA Hirofumi]

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

[PATCH] gitk: add some key-bind

[OGAWA Hirofumi]

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

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

[Paul Mackerras]

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.

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

[OGAWA Hirofumi]

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