{"thread":{"id":"14493","subject":"[PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories","startedAt":"2008-07-16T20:42:29Z","lastAt":"2008-07-18T05:39:05Z","messageCount":6,"participants":["Alexander Gavrilov","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"83583","messageId":"200807170042.29462.angavrilov@gmail.com","threadId":"14493","inReplyTo":null,"subject":"[PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2008-07-16T20:42:29Z","receivedAt":"2008-07-16T20:42:29Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"Full copy detection can take quite some time on large repositories, which\nsubstantially decreases perceived usability of the 'git gui blame' command.\nThis set of patches tries to overcome it by:\n\n1) Allowing the user to disable '-C -C' and/or set the detection threshold.\n\n2) Explicitly killing back-end processes, which don't produce any output\n  during copy detection, and thus normally won't receive SIGPIPE until\n  it is finished. Runaway processes are annoying.\n\n3) To compensate for (1), adding a context menu item to manually invoke\n  blame -C -C -C on a group of lines.\n\nAlexander Gavrilov (3):\n      Add options to control the search for copies in blame.\n      Kill the blame back-end on window close.\n      Add a menu item to invoke full copy detection in blame.\n\n git-gui.sh     |   16 ++++++++\n lib/blame.tcl  |  105 +++++++++++++++++++++++++++++++++++++++++++++++++------\n lib/option.tcl |    2 +\n 3 files changed, 111 insertions(+), 12 deletions(-)\n"},{"id":"83584","messageId":"200807170043.49016.angavrilov@gmail.com","threadId":"14493","inReplyTo":"200807170042.29462.angavrilov@gmail.com","subject":"[PATCH (GIT-GUI) 1/3] Add options to control the search for copies in blame.","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2008-07-16T20:43:48Z","receivedAt":"2008-07-16T20:43:48Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"On huge repositories, -C -C can be way too slow to be\nunconditionally enabled, and it can also be useful to control\nits precision.\n\nSigned-off-by: Alexander Gavrilov <angavrilov@gmail.com>\n---\n git-gui.sh     |    2 ++\n lib/blame.tcl  |   20 ++++++++++++--------\n lib/option.tcl |    2 ++\n 3 files changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex e3b6669..b1ed0ec 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -642,6 +642,8 @@ set default_config(user.email) {}\n set default_config(gui.matchtrackingbranch) false\n set default_config(gui.pruneduringfetch) false\n set default_config(gui.trustmtime) false\n+set default_config(gui.fastcopyblame) false\n+set default_config(gui.copyblamethreshold) 40\n set default_config(gui.diffcontext) 5\n set default_config(gui.commitmsgwidth) 75\n set default_config(gui.newbranchtemplate) {}\ndiff --git a/lib/blame.tcl b/lib/blame.tcl\nindex 92fac1b..192505d 100644\n--- a/lib/blame.tcl\n+++ b/lib/blame.tcl\n@@ -33,13 +33,6 @@ variable group_colors {\n \t#ececec\n }\n \n-# Switches for original location detection\n-#\n-variable original_options [list -C -C]\n-if {[git-version >= 1.5.3]} {\n-\tlappend original_options -w ; # ignore indentation changes\n-}\n-\n # Current blame data; cleared/reset on each load\n #\n field commit               ; # input commit to blame\n@@ -511,7 +504,6 @@ method _exec_blame {cur_w cur_d options cur_s} {\n method _read_blame {fd cur_w cur_d} {\n \tupvar #0 $cur_d line_data\n \tvariable group_colors\n-\tvariable original_options\n \n \tif {$fd ne $current_fd} {\n \t\tcatch {close $fd}\n@@ -684,6 +676,18 @@ method _read_blame {fd cur_w cur_d} {\n \tif {[eof $fd]} {\n \t\tclose $fd\n \t\tif {$cur_w eq $w_asim} {\n+\t\t\t# Switches for original location detection\n+\t\t\tset threshold [get_config gui.copyblamethreshold]\n+\t\t\tset original_options [list \"-C$threshold\"]\n+\n+\t\t\tif {![is_config_true gui.fastcopyblame]} {\n+\t\t\t\t# thorough copy search; insert before the threshold\n+\t\t\t\tset original_options [linsert $original_options 0 -C]\n+\t\t\t}\n+\t\t\tif {[git-version >= 1.5.3]} {\n+\t\t\t\tlappend original_options -w ; # ignore indentation changes\n+\t\t\t}\n+\n \t\t\t_exec_blame $this $w_amov @amov_data \\\n \t\t\t\t$original_options \\\n \t\t\t\t[mc \"Loading original location annotations...\"]\ndiff --git a/lib/option.tcl b/lib/option.tcl\nindex 9270512..ffb3f00 100644\n--- a/lib/option.tcl\n+++ b/lib/option.tcl\n@@ -123,6 +123,8 @@ proc do_options {} {\n \t\t{b gui.trustmtime  {mc \"Trust File Modification Timestamps\"}}\n \t\t{b gui.pruneduringfetch {mc \"Prune Tracking Branches During Fetch\"}}\n \t\t{b gui.matchtrackingbranch {mc \"Match Tracking Branches\"}}\n+\t\t{b gui.fastcopyblame {mc \"Blame Copy Only On Changed Files\"}}\n+\t\t{i-20..200 gui.copyblamethreshold {mc \"Minimum Letters To Blame Copy On\"}}\n \t\t{i-0..99 gui.diffcontext {mc \"Number of Diff Context Lines\"}}\n \t\t{i-0..99 gui.commitmsgwidth {mc \"Commit Message Text Width\"}}\n \t\t{t gui.newbranchtemplate {mc \"New Branch Name Template\"}}\n-- \n1.5.6.3.17.g3f148\n"},{"id":"83592","messageId":"200807170048.08909.angavrilov@gmail.com","threadId":"14493","inReplyTo":"200807170043.49016.angavrilov@gmail.com","subject":"[PATCH (GIT-GUI) 2/3] Kill the blame back-end on window close.","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2008-07-16T20:48:08Z","receivedAt":"2008-07-16T20:48:08Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"Currently 'git-gui blame' does not kill its back-end\nprocess, hoping that it will die anyway when the pipe\nis closed. However, in some cases the process works\nfor a long time without producing any output. This\nbehavior results in a runaway CPU hog.\n\nSigned-off-by: Alexander Gavrilov <angavrilov@gmail.com>\n---\n\n\tThe -f flag is necessary for msysgit.\n\n\tFor this fix I submitted to msysgit a patch that includes\n\ta Cygwin-compatible kill.exe in the installer.\n\n\t-- Alexander\n\n git-gui.sh    |   14 ++++++++++++++\n lib/blame.tcl |   16 ++++++++++++----\n 2 files changed, 26 insertions(+), 4 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex b1ed0ec..83e2645 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -497,6 +497,20 @@ proc githook_read {hook_name args} {\n \treturn {}\n }\n \n+proc kill_file_process {fd} {\n+\tset process [pid $fd]\n+\n+\tcatch {\n+\t\tif {[is_Windows]} {\n+\t\t\t# Use a Cygwin-specific flag to allow killing\n+\t\t\t# native Windows processes\n+\t\t\texec kill -f $process\n+\t\t} else {\n+\t\t\texec kill $process\n+\t\t}\n+\t}\n+}\n+\n proc sq {value} {\n \tregsub -all ' $value \"'\\\\''\" value\n \treturn \"'$value'\"\ndiff --git a/lib/blame.tcl b/lib/blame.tcl\nindex 192505d..2c19048 100644\n--- a/lib/blame.tcl\n+++ b/lib/blame.tcl\n@@ -326,19 +326,27 @@ constructor new {i_commit i_path} {\n \tbind $w.file_pane <Configure> \\\n \t\"if {{$w.file_pane} eq {%W}} {[cb _resize %h]}\"\n \n+\twm protocol $top WM_DELETE_WINDOW \"destroy $top\"\n+\tbind $top <Destroy> [cb _kill]\n+\n \t_load $this {}\n }\n \n+method _kill {} {\n+\tif {$current_fd ne {}} {\n+\t\tkill_file_process $current_fd\n+\t\tcatch {close $current_fd}\n+\t\tset current_fd {}\n+\t}\n+}\n+\n method _load {jump} {\n \tvariable group_colors\n \n \t_hide_tooltip $this\n \n \tif {$total_lines != 0 || $current_fd ne {}} {\n-\t\tif {$current_fd ne {}} {\n-\t\t\tcatch {close $current_fd}\n-\t\t\tset current_fd {}\n-\t\t}\n+\t\t_kill $this\n \n \t\tforeach i $w_columns {\n \t\t\t$i conf -state normal\n-- \n1.5.6.3.17.g3f148\n"},{"id":"83593","messageId":"200807170051.20337.angavrilov@gmail.com","threadId":"14493","inReplyTo":"200807170048.08909.angavrilov@gmail.com","subject":"[PATCH (GIT-GUI) 3/3] Add a menu item to invoke full copy detection in blame.","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2008-07-16T20:51:20Z","receivedAt":"2008-07-16T20:51:20Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"Add a context menu item to invoke blame -C -C -C on a chunk\nof the file. The results are used to update the 'original\nlocation' column of the blame display.\n\nThe chunk is computed as the smallest line range that covers\nboth the 'last change' and 'original location' ranges of the\nline that was clicked to open the menu.\n\nSigned-off-by: Alexander Gavrilov <angavrilov@gmail.com>\n---\n\n\tThis is my most complex Tcl/Tk code so far, so I might have\n\tdone some stupid things.\n\n\t-- Alexander\n\n lib/blame.tcl |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 69 insertions(+), 0 deletions(-)\n\ndiff --git a/lib/blame.tcl b/lib/blame.tcl\nindex 2c19048..b6e42cb 100644\n--- a/lib/blame.tcl\n+++ b/lib/blame.tcl\n@@ -256,6 +256,9 @@ constructor new {i_commit i_path} {\n \t$w.ctxm add command \\\n \t\t-label [mc \"Copy Commit\"] \\\n \t\t-command [cb _copycommit]\n+\t$w.ctxm add command \\\n+\t\t-label [mc \"Do Full Copy Detection\"] \\\n+\t\t-command [cb _fullcopyblame]\n \n \tforeach i $w_columns {\n \t\tfor {set g 0} {$g < [llength $group_colors]} {incr g} {\n@@ -708,6 +711,72 @@ method _read_blame {fd cur_w cur_d} {\n \t}\n } ifdeleted { catch {close $fd} }\n \n+method _find_commit_bound {data_list start_idx delta} {\n+\tupvar #0 $data_list line_data\n+\tset pos $start_idx\n+\tset limit       [expr {[llength $line_data] - 1}]\n+\tset base_commit [lindex $line_data $pos 0]\n+\n+\twhile {$pos > 0 && $pos < $limit} {\n+\t\tset new_pos [expr {$pos + $delta}]\n+\t\tif {[lindex $line_data $new_pos 0] ne $base_commit} {\n+\t\t\treturn $pos\n+\t\t}\n+\n+\t\tset pos $new_pos\n+\t}\n+\n+\treturn $pos\n+}\n+\n+method _fullcopyblame {} {\n+\tif {$current_fd ne {}} {\n+\t\ttk_messageBox \\\n+\t\t\t-icon error \\\n+\t\t\t-type ok \\\n+\t\t\t-title [mc \"Busy\"] \\\n+\t\t\t-message [mc \"Annotation process is already running.\"]\n+\n+\t\treturn\n+\t}\n+\n+\t# Switches for original location detection\n+\tset threshold [get_config gui.copyblamethreshold]\n+\tset original_options [list -C -C \"-C$threshold\"]\n+\n+\tif {[git-version >= 1.5.3]} {\n+\t\tlappend original_options -w ; # ignore indentation changes\n+\t}\n+\n+\t# Find the line range\n+\tset pos @$::cursorX,$::cursorY\n+\tset lno [lindex [split [$::cursorW index $pos] .] 0]\n+\tset min_amov_lno [_find_commit_bound $this @amov_data $lno -1]\n+\tset max_amov_lno [_find_commit_bound $this @amov_data $lno 1]\n+\tset min_asim_lno [_find_commit_bound $this @asim_data $lno -1]\n+\tset max_asim_lno [_find_commit_bound $this @asim_data $lno 1]\n+\n+\tif {$min_asim_lno < $min_amov_lno} {\n+\t\tset min_amov_lno $min_asim_lno\n+\t}\n+\n+\tif {$max_asim_lno > $max_amov_lno} {\n+\t\tset max_amov_lno $max_asim_lno\n+\t}\n+\n+\tlappend original_options -L \"$min_amov_lno,$max_amov_lno\"\n+\n+\t# Clear lines\n+\tfor {set i $min_amov_lno} {$i <= $max_amov_lno} {incr i} {\n+\t\tlset amov_data $i [list ]\n+\t}\n+\n+\t# Start the back-end process\n+\t_exec_blame $this $w_amov @amov_data \\\n+\t\t$original_options \\\n+\t\t[mc \"Running thorough copy detection...\"]\n+}\n+\n method _click {cur_w pos} {\n \tset lno [lindex [split [$cur_w index $pos] .] 0]\n \t_showcommit $this $cur_w $lno\n-- \n1.5.6.3.17.g3f148\n"},{"id":"83634","messageId":"20080717021623.GC16740@spearce.org","threadId":"14493","inReplyTo":"200807170042.29462.angavrilov@gmail.com","subject":"Re: [PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-17T02:16:23Z","receivedAt":"2008-07-17T02:16:23Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Alexander Gavrilov <angavrilov@gmail.com> wrote:\n> Full copy detection can take quite some time on large repositories, which\n> substantially decreases perceived usability of the 'git gui blame' command.\n> This set of patches tries to overcome it by:\n> \n> 1) Allowing the user to disable '-C -C' and/or set the detection threshold.\n> \n> 2) Explicitly killing back-end processes, which don't produce any output\n>   during copy detection, and thus normally won't receive SIGPIPE until\n>   it is finished. Runaway processes are annoying.\n> \n> 3) To compensate for (1), adding a context menu item to manually invoke\n>   blame -C -C -C on a group of lines.\n\nThis series is nicely done.  Works well on revision.c in git.git,\nwhere the blame can be costly to compute with full copy detection.\nAnd getting the incremental from the context menu also works well.\n\nThanks.\n \n-- \nShawn.\n"},{"id":"83820","messageId":"200807180939.06082.angavrilov@gmail.com","threadId":"14493","inReplyTo":"20080717021623.GC16740@spearce.org","subject":"Re: [PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories","fromName":"Alexander Gavrilov","fromEmail":"angavrilov@gmail.com","sentAt":"2008-07-18T05:39:05Z","receivedAt":"2008-07-18T05:39:05Z","isPatch":true,"sender":{"key":"angavrilov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42666?v=4"},"body":"Hello,\n\nOn Thursday 17 July 2008 06:16:23 Shawn O. Pearce wrote:\n> Alexander Gavrilov <angavrilov@gmail.com> wrote:\n> > Full copy detection can take quite some time on large repositories, which\n> > substantially decreases perceived usability of the 'git gui blame' command.\n> > This set of patches tries to overcome it by:\n> > \n> > 1) Allowing the user to disable '-C -C' and/or set the detection threshold.\n> > \n> > 2) Explicitly killing back-end processes, which don't produce any output\n> >   during copy detection, and thus normally won't receive SIGPIPE until\n> >   it is finished. Runaway processes are annoying.\n> > \n> > 3) To compensate for (1), adding a context menu item to manually invoke\n> >   blame -C -C -C on a group of lines.\n> \n> This series is nicely done.  Works well on revision.c in git.git,\n> where the blame can be costly to compute with full copy detection.\n> And getting the incremental from the context menu also works well.\n\nI also thought that it would be useful to be able to specify the line range explicitly,\nbut couldn't invent a good UI for it. Selecting lines with the mouse also causes a\ncommit to be selected and recolored in green, and popping up a dialog to request\nline numbers seems too lame.\n\nAlexander\n"}]}