{"thread":{"id":"12720","subject":"[PATCH] gitk: synchronize highlighting in file view for 'f'&'b' commands","startedAt":"2008-03-17T02:50:44Z","lastAt":"2008-03-17T07:42:05Z","messageCount":2,"participants":["Eric Raible","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"72269","messageId":"279b37b20803161950q73853290w864eda69896efb9a@mail.gmail.com","threadId":"12720","inReplyTo":null,"subject":"[PATCH] gitk: synchronize highlighting in file view for 'f'&'b' commands","fromName":"Eric Raible","fromEmail":"raible@gmail.com","sentAt":"2008-03-17T02:50:44Z","receivedAt":"2008-03-17T02:50:44Z","isPatch":true,"sender":{"key":"raible@gmail.com","avatar":null},"body":">From 03603a207b2caad8b52dd4a24860e387d418f800 Mon Sep 17 00:00:00 2001\nFrom: Eric Raible <raible+git@gmail.com>\nDate: Sun, 16 Mar 2008 19:33:52 -0700\nSubject: [PATCH] gitk: synchronize highlighting in file view for\n'f'&'b' commands\n\nPreviously, 'b', backspace, and delete all did the same thing.\nThis changes 'b' to perform the inverse of 'f'.  And both of\nthem now highlight the filename of the currently diff.\n\nThis makes it easier to review the diffs associated with\na particular commit using only f,b, and space.\n\nSigned-off-by: Eric Raible <raible+git@gmail.com>\n---\n gitk-git/gitk |   23 ++++++++++++++++++++---\n 1 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 84ab02e..e9ca6f7 100644\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -1016,7 +1016,7 @@ proc makewindow {} {\n     bindkey k \"selnextline 1\"\n     bindkey j \"goback\"\n     bindkey l \"goforw\"\n-    bindkey b \"$ctext yview scroll -1 pages\"\n+    bindkey b prevfile\n     bindkey d \"$ctext yview scroll 18 units\"\n     bindkey u \"$ctext yview scroll -18 units\"\n     bindkey / {dofind 1 1}\n@@ -5478,13 +5478,30 @@ proc changediffdisp {} {\n     $ctext tag conf d1 -elide [lindex $diffelide 1]\n }\n\n+proc highlightfile {loc} {\n+    global ctext\n+    $ctext yview $loc\n+\n+    global cmitmode ctext cflist cflist_top\n+    if {$cmitmode eq \"tree\"} return\n+\n+    $cflist tag remove highlight $cflist_top.0 \"$cflist_top.0 lineend\"\n+\n+    set file [regsub -- \"-* (.*?) -*\" [$ctext get $loc \"$loc lineend\"] \"\\\\1\"]\n+    set cline [$cflist search -regexp [subst {^$file$}] 0.0]\n+\n+    $cflist tag add highlight $cline \"$cline lineend\"\n+    $cflist see $cline\n+    set cflist_top [lindex [split $cline .] 0]\n+}\n+\n proc prevfile {} {\n     global difffilestart ctext\n     set prev [lindex $difffilestart 0]\n     set here [$ctext index @0,0]\n     foreach loc $difffilestart {\n \tif {[$ctext compare $loc >= $here]} {\n-\t    $ctext yview $prev\n+\t    highlightfile $prev\n \t    return\n \t}\n \tset prev $loc\n@@ -5497,7 +5514,7 @@ proc nextfile {} {\n     set here [$ctext index @0,0]\n     foreach loc $difffilestart {\n \tif {[$ctext compare $loc > $here]} {\n-\t    $ctext yview $loc\n+\t    highlightfile $loc\n \t    return\n \t}\n     }\n-- \n1.5.4.2.1161.g1a6f0\n"},{"id":"72270","messageId":"7v63vldcfm.fsf@gitster.siamese.dyndns.org","threadId":"12720","inReplyTo":"279b37b20803161950q73853290w864eda69896efb9a@mail.gmail.com","subject":"Re: [PATCH] gitk: synchronize highlighting in file view for 'f'&'b' commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-17T07:42:05Z","receivedAt":"2008-03-17T07:42:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eric Raible\" <raible@gmail.com> writes:\n\n> From 03603a207b2caad8b52dd4a24860e387d418f800 Mon Sep 17 00:00:00 2001\n> From: Eric Raible <raible+git@gmail.com>\n> Date: Sun, 16 Mar 2008 19:33:52 -0700\n> Subject: [PATCH] gitk: synchronize highlighting in file view for\n> 'f'&'b' commands\n\nPlease do not do this.  The first line is not part of anything but is only\na mail message boundary in mbox format.  Reproducing From: is fine if the\npatch author is different from the person who is sending the patch, but I\ndo not think it is necessary in this case.  Date and Subject should also\ngo, as taking them from the e-mail header is just as good.\n\nAnd please do not send gitk patches to me.  It should come through Paul\nMackerras <paulus@samba.org>, so if your patch is obviously correct for\napplication, then To: him with Cc: list (and I do not mind being on the\nCc:), otherwise if the patch is for review and discussion, please send it\naddressed To: the list (and again I do not mind being on the Cc:).\n\n> Previously, 'b', backspace, and delete all did the same thing.\n> This changes 'b' to perform the inverse of 'f'.  And both of\n> them now highlight the filename of the currently diff.\n>\n> This makes it easier to review the diffs associated with\n> a particular commit using only f,b, and space.\n\nAs to the change itself, I think it makes sense to have a binding\navailable for prevfile (previously there was none), so I'd be supportive\nof that keybinding change.\n\n> +proc highlightfile {loc} {\n> +    global ctext\n> +    $ctext yview $loc\n> +\n> +    global cmitmode ctext cflist cflist_top\n> +    if {$cmitmode eq \"tree\"} return\n> +\n> +    $cflist tag remove highlight $cflist_top.0 \"$cflist_top.0 lineend\"\n> +\n> +    set file [regsub -- \"-* (.*?) -*\" [$ctext get $loc \"$loc lineend\"] \"\\\\1\"]\n> +    set cline [$cflist search -regexp [subst {^$file$}] 0.0]\n> +\n> +    $cflist tag add highlight $cline \"$cline lineend\"\n> +    $cflist see $cline\n> +    set cflist_top [lindex [split $cline .] 0]\n> +}\n\nI however think you also should describe why you had to change \"$ctext\nyview\" in both nextfile and prevfile to a more complex \"highlightfile\" in\nyour commit message.  It is not very obvious how it is an improvement, and\ntyping 'b' in tree view (as opposed to patch view) seems to trigger\n\n\tError: bad text index\"\"\n\nwith this patch.\n"}]}