* [PATCH] gitk: use single blamestuff for all show_line_source{} calls @ 2014-02-03 20:53 Max Kirillov 2014-02-03 22:34 ` [PATCH 0/3] gitk: show latest change to region Max Kirillov 0 siblings, 1 reply; 8+ messages in thread From: Max Kirillov @ 2014-02-03 20:53 UTC (permalink / raw) To: Paul Mackerras; +Cc: git There seems to be no point to search for several origins at once. I doubt it is even fully working (because there is one blameinst), but blamestuff for some reason is an array. Also, it is not cleaned after blame is completed Signed-off-by: Max Kirillov <max@max630.net> --- gitk | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/gitk b/gitk index 90764e8..dfac4fd 100755 --- a/gitk +++ b/gitk @@ -3815,17 +3815,18 @@ proc show_line_source {} { nowbusy blaming [mc "Searching"] fconfigure $f -blocking 0 set i [reg_instance $f] - set blamestuff($i) {} + set blamestuff {} set blameinst $i filerun $f [list read_line_source $f $i] } proc stopblaming {} { - global blameinst + global blameinst blamestuff if {[info exists blameinst]} { stop_instance $blameinst unset blameinst + unset blamestuff notbusy blaming } } @@ -3834,7 +3835,7 @@ proc read_line_source {fd inst} { global blamestuff curview commfd blameinst nullid nullid2 while {[gets $fd line] >= 0} { - lappend blamestuff($inst) $line + lappend blamestuff $line } if {![eof $fd]} { return 1 @@ -3845,17 +3846,18 @@ proc read_line_source {fd inst} { fconfigure $fd -blocking 1 if {[catch {close $fd} err]} { error_popup [mc "Error running git blame: %s" $err] + unset blamestuff return 0 } set fname {} - set line [split [lindex $blamestuff($inst) 0] " "] + set line [split [lindex $blamestuff 0] " "] set id [lindex $line 0] set lnum [lindex $line 1] if {[string length $id] == 40 && [string is xdigit $id] && [string is digit -strict $lnum]} { # look for "filename" line - foreach l $blamestuff($inst) { + foreach l $blamestuff { if {[string match "filename *" $l]} { set fname [string range $l 9 end] break @@ -3878,6 +3880,7 @@ proc read_line_source {fd inst} { } else { puts "oops couldn't parse git blame output" } + unset blamestuff return 0 } -- 1.8.5.2.421.g4cdf8d0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 0/3] gitk: show latest change to region 2014-02-03 20:53 [PATCH] gitk: use single blamestuff for all show_line_source{} calls Max Kirillov @ 2014-02-03 22:34 ` Max Kirillov 2014-02-03 22:41 ` [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} Max Kirillov ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Max Kirillov @ 2014-02-03 22:34 UTC (permalink / raw) To: Paul Mackerras; +Cc: git Hi! I quite like the "Show origin of this line" feature of the gitk. It is more convenient than blame, because it directly answers the question which is usually addressed to blame. But, sometimes there is no "key" line which one could blame. Instead there is a function, block, or some other region of code. So I need a similar feature, but for several lines rather than for a single one. The series of patches implements the feature. It consists of 3 patches: * gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} * gitk: refactor: separate io from logic in the searching origin of line * gitk: pick selection for region blame It also requires the "gitk: use single blamestuff for all show_line_source{} calls" patch, which it replies to. -- Max ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} 2014-02-03 22:34 ` [PATCH 0/3] gitk: show latest change to region Max Kirillov @ 2014-02-03 22:41 ` Max Kirillov 2014-02-03 23:20 ` Eric Sunshine 2014-02-03 22:42 ` [PATCH 2/3] gitk: refactor: separate io from logic in the searching origin of line Max Kirillov 2014-02-03 22:42 ` [PATCH 3/3] gitk: pick selection for region blame Max Kirillov 2 siblings, 1 reply; 8+ messages in thread From: Max Kirillov @ 2014-02-03 22:41 UTC (permalink / raw) To: Paul Mackerras; +Cc: git For requesting a region blame, it is necessary to parse a hunk and find the region in the parent file corresponding to the selected region. There is already hunk parsin functionality in the find_hunk_blamespec{}, but returns only information for a single line. The new function, resolve_hunk_lines{}, scans the hunk once and returns for all hunk lines between $start_diffline and $end_diffline, in which parent each of them exists and which is its number there. Signed-off-by: Max Kirillov <max@max630.net> --- gitk | 93 ++++++++++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 36 deletions(-) diff --git a/gitk b/gitk index dfac4fd..7699a66 100755 --- a/gitk +++ b/gitk @@ -3590,11 +3590,11 @@ proc external_diff {} { } } -proc find_hunk_blamespec {base line} { +proc resolve_hunk_lines {base start_diffline end_diffline} { global ctext # Find and parse the hunk header - set s_lix [$ctext search -backwards -regexp ^@@ "$line.0 lineend" $base.0] + set s_lix [$ctext search -backwards -regexp ^@@ "$start_diffline.0 lineend" $base.0] if {$s_lix eq {}} return set s_line [$ctext get $s_lix "$s_lix + 1 lines"] @@ -3614,49 +3614,70 @@ proc find_hunk_blamespec {base line} { } # Now scan the lines to determine offset within the hunk - set max_parent [expr {[llength $base_lines]-2}] - set dline 0 + set max_parent [expr {[llength $base_lines]-1}] set s_lno [lindex [split $s_lix "."] 0] - # Determine if the line is removed - set chunk [$ctext get $line.0 "$line.1 + $max_parent chars"] - if {[string match {[-+ ]*} $chunk]} { - set removed_idx [string first "-" $chunk] - # Choose a parent index - if {$removed_idx >= 0} { - set parent $removed_idx + set commitlines_by_diffline {} + array unset commit_lines + for {set p 0} {$p <= $max_parent} {incr p} { + set commit_lines($p) [expr [lindex $base_lines $p] - 1] + } + for {set diffline [expr $s_lno + 1]} {$diffline <= $end_diffline} {incr diffline} { + set chunk [$ctext get $diffline.0 "$diffline.0 + $max_parent chars"] + if {$chunk eq {} || [string match "\[\n@\]*" $chunk]} { + # region is larger than hunk + return {} + } + set is_removed [expr [string first "-" $chunk] >= 0] + if {!$is_removed} { + incr commit_lines(0) + set commitlines [list [list 0 $commit_lines(0)]] } else { - set unchanged_idx [string first " " $chunk] - if {$unchanged_idx >= 0} { - set parent $unchanged_idx - } else { - # blame the current commit - set parent -1 - } - } - # then count other lines that belong to it - for {set i $line} {[incr i -1] > $s_lno} {} { - set chunk [$ctext get $i.0 "$i.1 + $max_parent chars"] - # Determine if the line is removed - set removed_idx [string first "-" $chunk] - if {$parent >= 0} { - set code [string index $chunk $parent] - if {$code eq "-" || ($removed_idx < 0 && $code ne "+")} { - incr dline + set commitlines {} + } + for {set p 1} {$p <= $max_parent} {incr p} { + switch -- [string index $chunk "$p-1"] { + "+" { } - } else { - if {$removed_idx < 0} { - incr dline + "-" { + incr commit_lines($p) + lappend commitlines [list $p $commit_lines($p)] + } + " " { + if {!$is_removed} { + incr commit_lines($p) + lappend commitlines [list $p $commit_lines($p)] + } + } + default { + error_popup "resolve_hunk_lines: unexpected diff line($diffline): $chunk" + break } } } - incr parent - } else { - set parent 0 + if {$diffline >= $start_diffline} { + lappend commitlines_by_diffline [list $diffline $commitlines] + } } + return $commitlines_by_diffline +} - incr dline [lindex $base_lines $parent] - return [list $parent $dline] +proc find_hunk_blamespec {base line} { + foreach cl_spec [resolve_hunk_lines $base $line $line] { + if {[lindex $cl_spec 0] == $line} { + set commitlines [lindex $cl_spec 1] + if {[llength $commitlines] > 0} { + if {[llength $commitlines] > 1 && [lindex $commitlines 0 0] eq 0} { + return [lindex $commitlines 1] + } else { + return [lindex $commitlines 0] + } + } else { + error_popup "find_hunk_blamespec: invalid commitlines: $commitlines" + } + } + } + return {} } proc external_blame_diff {} { -- 1.8.5.2.421.g4cdf8d0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} 2014-02-03 22:41 ` [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} Max Kirillov @ 2014-02-03 23:20 ` Eric Sunshine 2014-06-24 18:27 ` Max Kirillov 0 siblings, 1 reply; 8+ messages in thread From: Eric Sunshine @ 2014-02-03 23:20 UTC (permalink / raw) To: Max Kirillov; +Cc: Paul Mackerras, Git List On Mon, Feb 3, 2014 at 5:41 PM, Max Kirillov <max@max630.net> wrote: > For requesting a region blame, it is necessary to parse a hunk and > find the region in the parent file corresponding to the selected region. > There is already hunk parsin functionality in the find_hunk_blamespec{}, s/parsin/parsing/ s/in the/in/ > but returns only information for a single line. s/but/but it/ > The new function, resolve_hunk_lines{}, scans the hunk once and returns > for all hunk lines between $start_diffline and $end_diffline, in which parent > each of them exists and which is its number there. > > Signed-off-by: Max Kirillov <max@max630.net> > --- > gitk | 93 ++++++++++++++++++++++++++++++++++++++++++-------------------------- > 1 file changed, 57 insertions(+), 36 deletions(-) > > diff --git a/gitk b/gitk > index dfac4fd..7699a66 100755 > --- a/gitk > +++ b/gitk > @@ -3590,11 +3590,11 @@ proc external_diff {} { > } > } > > -proc find_hunk_blamespec {base line} { > +proc resolve_hunk_lines {base start_diffline end_diffline} { > global ctext > > # Find and parse the hunk header > - set s_lix [$ctext search -backwards -regexp ^@@ "$line.0 lineend" $base.0] > + set s_lix [$ctext search -backwards -regexp ^@@ "$start_diffline.0 lineend" $base.0] > if {$s_lix eq {}} return > > set s_line [$ctext get $s_lix "$s_lix + 1 lines"] > @@ -3614,49 +3614,70 @@ proc find_hunk_blamespec {base line} { > } > > # Now scan the lines to determine offset within the hunk > - set max_parent [expr {[llength $base_lines]-2}] > - set dline 0 > + set max_parent [expr {[llength $base_lines]-1}] > set s_lno [lindex [split $s_lix "."] 0] > > - # Determine if the line is removed > - set chunk [$ctext get $line.0 "$line.1 + $max_parent chars"] > - if {[string match {[-+ ]*} $chunk]} { > - set removed_idx [string first "-" $chunk] > - # Choose a parent index > - if {$removed_idx >= 0} { > - set parent $removed_idx > + set commitlines_by_diffline {} > + array unset commit_lines > + for {set p 0} {$p <= $max_parent} {incr p} { > + set commit_lines($p) [expr [lindex $base_lines $p] - 1] > + } > + for {set diffline [expr $s_lno + 1]} {$diffline <= $end_diffline} {incr diffline} { > + set chunk [$ctext get $diffline.0 "$diffline.0 + $max_parent chars"] > + if {$chunk eq {} || [string match "\[\n@\]*" $chunk]} { > + # region is larger than hunk > + return {} > + } > + set is_removed [expr [string first "-" $chunk] >= 0] > + if {!$is_removed} { > + incr commit_lines(0) > + set commitlines [list [list 0 $commit_lines(0)]] > } else { > - set unchanged_idx [string first " " $chunk] > - if {$unchanged_idx >= 0} { > - set parent $unchanged_idx > - } else { > - # blame the current commit > - set parent -1 > - } > - } > - # then count other lines that belong to it > - for {set i $line} {[incr i -1] > $s_lno} {} { > - set chunk [$ctext get $i.0 "$i.1 + $max_parent chars"] > - # Determine if the line is removed > - set removed_idx [string first "-" $chunk] > - if {$parent >= 0} { > - set code [string index $chunk $parent] > - if {$code eq "-" || ($removed_idx < 0 && $code ne "+")} { > - incr dline > + set commitlines {} > + } > + for {set p 1} {$p <= $max_parent} {incr p} { > + switch -- [string index $chunk "$p-1"] { > + "+" { > } > - } else { > - if {$removed_idx < 0} { > - incr dline > + "-" { > + incr commit_lines($p) > + lappend commitlines [list $p $commit_lines($p)] > + } > + " " { > + if {!$is_removed} { > + incr commit_lines($p) > + lappend commitlines [list $p $commit_lines($p)] > + } > + } > + default { > + error_popup "resolve_hunk_lines: unexpected diff line($diffline): $chunk" > + break > } > } > } > - incr parent > - } else { > - set parent 0 > + if {$diffline >= $start_diffline} { > + lappend commitlines_by_diffline [list $diffline $commitlines] > + } > } > + return $commitlines_by_diffline > +} > > - incr dline [lindex $base_lines $parent] > - return [list $parent $dline] > +proc find_hunk_blamespec {base line} { > + foreach cl_spec [resolve_hunk_lines $base $line $line] { > + if {[lindex $cl_spec 0] == $line} { > + set commitlines [lindex $cl_spec 1] > + if {[llength $commitlines] > 0} { > + if {[llength $commitlines] > 1 && [lindex $commitlines 0 0] eq 0} { > + return [lindex $commitlines 1] > + } else { > + return [lindex $commitlines 0] > + } > + } else { > + error_popup "find_hunk_blamespec: invalid commitlines: $commitlines" > + } > + } > + } > + return {} > } > > proc external_blame_diff {} { > -- > 1.8.5.2.421.g4cdf8d0 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} 2014-02-03 23:20 ` Eric Sunshine @ 2014-06-24 18:27 ` Max Kirillov 0 siblings, 0 replies; 8+ messages in thread From: Max Kirillov @ 2014-06-24 18:27 UTC (permalink / raw) To: Eric Sunshine; +Cc: Paul Mackerras, Git List On Mon, Feb 03, 2014 at 06:20:36PM -0500, Eric Sunshine wrote: > On Mon, Feb 3, 2014 at 5:41 PM, Max Kirillov <max@max630.net> wrote: >> For requesting a region blame, it is necessary to parse a hunk and >> find the region in the parent file corresponding to the selected region. >> There is already hunk parsin functionality in the find_hunk_blamespec{}, > > s/parsin/parsing/ > s/in the/in/ > >> but returns only information for a single line. > > s/but/but it/ > Thank you! -- Max ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] gitk: refactor: separate io from logic in the searching origin of line 2014-02-03 22:34 ` [PATCH 0/3] gitk: show latest change to region Max Kirillov 2014-02-03 22:41 ` [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} Max Kirillov @ 2014-02-03 22:42 ` Max Kirillov 2014-02-03 22:42 ` [PATCH 3/3] gitk: pick selection for region blame Max Kirillov 2 siblings, 0 replies; 8+ messages in thread From: Max Kirillov @ 2014-02-03 22:42 UTC (permalink / raw) To: Paul Mackerras; +Cc: git The pattern of maintaining blame command and collecting output can be reused for searching of latest change to region. It still can use the blame's global variables, because the two search commands should not run concurrently as well as two instances of blame. Signed-off-by: Max Kirillov <max@max630.net> --- gitk | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/gitk b/gitk index 7699a66..eef88a1 100755 --- a/gitk +++ b/gitk @@ -3771,7 +3771,7 @@ proc external_blame {parent_idx {line {}}} { } proc show_line_source {} { - global cmitmode currentid parents curview blamestuff blameinst + global cmitmode currentid parents curview global diff_menu_line diff_menu_filebase flist_menu_file global nullid nullid2 gitdir cdup @@ -3827,6 +3827,12 @@ proc show_line_source {} { lappend blameargs $id } lappend blameargs -- [file join $cdup $flist_menu_file] + startblaming $blameargs read_line_source +} + +proc startblaming {blameargs blamecommand} { + global blamestuff blameinst + if {[catch { set f [open $blameargs r] } err]} { @@ -3838,7 +3844,7 @@ proc show_line_source {} { set i [reg_instance $f] set blamestuff {} set blameinst $i - filerun $f [list read_line_source $f $i] + filerun $f [list blameiocallback $f $i $blamecommand] } proc stopblaming {} { @@ -3852,8 +3858,8 @@ proc stopblaming {} { } } -proc read_line_source {fd inst} { - global blamestuff curview commfd blameinst nullid nullid2 +proc blameiocallback {fd inst blamecommand} { + global blamestuff blameinst commfd while {[gets $fd line] >= 0} { lappend blamestuff $line @@ -3871,6 +3877,14 @@ proc read_line_source {fd inst} { return 0 } + $blamecommand + unset blamestuff + return 0 +} + +proc read_line_source {} { + global blamestuff curview nullid nullid2 + set fname {} set line [split [lindex $blamestuff 0] " "] set id [lindex $line 0] @@ -3901,7 +3915,6 @@ proc read_line_source {fd inst} { } else { puts "oops couldn't parse git blame output" } - unset blamestuff return 0 } -- 1.8.5.2.421.g4cdf8d0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] gitk: pick selection for region blame 2014-02-03 22:34 ` [PATCH 0/3] gitk: show latest change to region Max Kirillov 2014-02-03 22:41 ` [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} Max Kirillov 2014-02-03 22:42 ` [PATCH 2/3] gitk: refactor: separate io from logic in the searching origin of line Max Kirillov @ 2014-02-03 22:42 ` Max Kirillov 2014-02-03 22:48 ` [PATCH 3/3 v2] gitk: show latest change to region Max Kirillov 2 siblings, 1 reply; 8+ messages in thread From: Max Kirillov @ 2014-02-03 22:42 UTC (permalink / raw) To: Paul Mackerras; +Cc: git Add the new command to the diffmenu, "Show the latest change of selected region". The menu command picks selection, and if it exists and covers a single hunk, locates the latest change which has been made to the selected lines in the file. The menu command is disabled if the region blame is impossible, for example if nothing is selected or the selection does not lie fully in a single diff hunk. The search is implemented by use of "git log -L..." command. Unlike git blame, it can locate merges which brings together changes to the region from different branches. This is what is desired, actually. Unfortunally, using git log -L for finding a single line origin is suboptimal, because (a) it does not support the "--contents" commandline argument, or any other way to blame uncommitted changes, and (b) it is noticeably slower. Hopely in some future git log -L will be mature enough to be used for picking the single line origin, for now the best option is to implement region logic separately, reusing their basic io. For diffs, the first parent is always searched. This decision is quite voluntary, just to avoid complications to UI. Signed-off-by: Max Kirillov <max@max630.net> --- gitk | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/gitk b/gitk index eef88a1..676c990 100755 --- a/gitk +++ b/gitk @@ -2650,6 +2650,7 @@ proc makewindow {} { makemenu $diff_menu { {mc "Show origin of this line" command show_line_source} {mc "Run git gui blame on this line" command {external_blame_diff}} + {mc "Show the latest change of selected region" command show_selection_source} } $diff_menu configure -tearoff 0 } @@ -3464,6 +3465,7 @@ proc pop_diff_menu {w X Y x y} { global ctext diff_menu flist_menu_file global diff_menu_txtpos diff_menu_line global diff_menu_filebase + global selection_source_data set diff_menu_txtpos [split [$w index "@$x,$y"] "."] set diff_menu_line [lindex $diff_menu_txtpos 0] @@ -3476,6 +3478,12 @@ proc pop_diff_menu {w X Y x y} { if {$f eq {}} return set flist_menu_file [lindex $f 0] set diff_menu_filebase [lindex $f 1] + prepare_show_selection_source + if {$selection_source_data ne {}} { + $diff_menu entryconf 2 -state normal + } else { + $diff_menu entryconf 2 -state disabled + } tk_popup $diff_menu $X $Y } @@ -3918,6 +3926,136 @@ proc read_line_source {} { return 0 } +proc show_selection_source {} { + global selection_source_data + global blameinst blamestuff + + if {$selection_source_data eq {}} { + return + } + + foreach var {id fname lnum lnumber} val $selection_source_data { set $var $val } + set args [list | git log --pretty=format:%H "-L$lnum,+$lnumber:$fname" -M -n1 $id] + + startblaming $args selsource_read +} + +proc selsource_read {} { + global blamestuff + global curview + + set id {} + set lnum {} + set state start + foreach l $blamestuff { + switch -- $state { + start { if {[regexp {^([0-9a-f]{40})$} $l _ id_match]} { + set id $id_match + set state diff_header_diff + } else { + break + } + } + diff_header_diff { if {[regexp {^diff --git} $l]} { set state diff_header_oldfile } else { break } } + diff_header_oldfile { if {[regexp {^---} $l]} {set state diff_header_newfile} else { break } } + diff_header_newfile { + if {[regexp {^\+\+\+ b/(.*)$} $l _ fname_match]} { + set fname $fname_match + set state diff_hunk_header + } else { + break + } + } + diff_hunk_header { + if {[regexp {^@@@*.* \+([0-9]+),[0-9]+ @@@*$} $l _ lnum_matched]} { + set lnum $lnum_matched + break + } + } + } + } + + if {$id eq {}} { + error_popup [mc "Parsing output of git log failed"] + return 0 + } + + if {![commitinview $id $curview]} { + error_popup [mc "The latest change is in commit %s, \ + which is not in this view" [shortids $id]] + return 0 + } + + selectline [rowofcommit $id] 1 [list $fname $lnum] +} + +proc prepare_show_selection_source {} { + global ctext + global selection_source_data + + set selection [$ctext tag ranges sel] + if {[llength $selection] != 2} { + set selection_source_data {} + return + } + set start_line [lindex [split [lindex $selection 0] "."] 0] + set end_index [split [lindex $selection 1] "."] + if {[lindex $end_index 1] eq 0} { + set end_line [expr [lindex $end_index 0] - 1] + } else { + set end_line [lindex $end_index 0] + } + set selection_source_data [prepare_show_region_source $start_line $end_line] +} + +proc prepare_show_region_source {start_line end_line} { + global cmitmode + global currentid parents curview + global nullid + if {$start_line > $end_line} { + error_popup "Cannot blame region: $start_line > $end_line" + return {} + } + set f [find_ctext_fileinfo $start_line] + if {$f eq {}} { + return {} + } + if {$currentid eq $nullid} { + return {} + } + set fname [lindex $f 0] + set filebase [lindex $f 1] + if {$cmitmode eq "tree"} { + set id $currentid + set start_lnum [expr $start_line - $filebase] + set end_lnum [expr $end_line - $filebase] + } else { + set id [lindex $parents($curview,$currentid) 0] + set start_lnum {} + set end_lnum {} + foreach cl_spec [resolve_hunk_lines $filebase $start_line $end_line] { + set diffline [lindex $cl_spec 0] + if {$diffline > $end_line} { + break + } elseif {$diffline >= $start_line} { + foreach branch [lindex $cl_spec 1] { + if {[lindex $branch 0] == 1} { + if {$start_lnum eq {}} { + set start_lnum [lindex $branch 1] + } + set end_lnum [lindex $branch 1] + } + } + } + } + } + if {$start_lnum ne {} && $end_lnum ne {}} { + return [list $id $fname $start_lnum [expr $end_lnum - $start_lnum + 1]] + } else { + return {} + } +} + # delete $dir when we see eof on $f (presumably because the child has exited) proc delete_at_eof {f dir} { while {[gets $f line] >= 0} {} -- 1.8.5.2.421.g4cdf8d0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3 v2] gitk: show latest change to region 2014-02-03 22:42 ` [PATCH 3/3] gitk: pick selection for region blame Max Kirillov @ 2014-02-03 22:48 ` Max Kirillov 0 siblings, 0 replies; 8+ messages in thread From: Max Kirillov @ 2014-02-03 22:48 UTC (permalink / raw) To: Paul Mackerras; +Cc: git Add a new command to the diffmenu, "Show the latest change of selected region". The menu command picks selection, and if it exists and covers a single hunk, locates the latest change which has been made to the selected lines in the file. The menu command is disabled if the region blame is impossible, for example if nothing is selected or the selection does not lie fully in a single diff hunk. The search is implemented by use of "git log -L..." command. Unlike git blame, it can locate merges which brings together changes to the region from different branches. This is what is desired, actually. Unfortunally, using git log -L for finding a single line origin is suboptimal, because (a) it does not support the "--contents" commandline argument, or any other way to blame uncommitted changes, and (b) it is noticeably slower. Hopely in some future git log -L will be mature enough to be used for picking the single line origin, for now the best option is to implement region logic separately, reusing the blame's basic io. For diffs, the first parent is always searched. This decision is quite voluntary, just to avoid complications to UI. Signed-off-by: Max Kirillov <max@max630.net> --- Fixed comment, same code gitk | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/gitk b/gitk index eef88a1..676c990 100755 --- a/gitk +++ b/gitk @@ -2650,6 +2650,7 @@ proc makewindow {} { makemenu $diff_menu { {mc "Show origin of this line" command show_line_source} {mc "Run git gui blame on this line" command {external_blame_diff}} + {mc "Show the latest change of selected region" command show_selection_source} } $diff_menu configure -tearoff 0 } @@ -3464,6 +3465,7 @@ proc pop_diff_menu {w X Y x y} { global ctext diff_menu flist_menu_file global diff_menu_txtpos diff_menu_line global diff_menu_filebase + global selection_source_data set diff_menu_txtpos [split [$w index "@$x,$y"] "."] set diff_menu_line [lindex $diff_menu_txtpos 0] @@ -3476,6 +3478,12 @@ proc pop_diff_menu {w X Y x y} { if {$f eq {}} return set flist_menu_file [lindex $f 0] set diff_menu_filebase [lindex $f 1] + prepare_show_selection_source + if {$selection_source_data ne {}} { + $diff_menu entryconf 2 -state normal + } else { + $diff_menu entryconf 2 -state disabled + } tk_popup $diff_menu $X $Y } @@ -3918,6 +3926,136 @@ proc read_line_source {} { return 0 } +proc show_selection_source {} { + global selection_source_data + global blameinst blamestuff + + if {$selection_source_data eq {}} { + return + } + + foreach var {id fname lnum lnumber} val $selection_source_data { set $var $val } + set args [list | git log --pretty=format:%H "-L$lnum,+$lnumber:$fname" -M -n1 $id] + + startblaming $args selsource_read +} + +proc selsource_read {} { + global blamestuff + global curview + + set id {} + set lnum {} + set state start + foreach l $blamestuff { + switch -- $state { + start { if {[regexp {^([0-9a-f]{40})$} $l _ id_match]} { + set id $id_match + set state diff_header_diff + } else { + break + } + } + diff_header_diff { if {[regexp {^diff --git} $l]} { set state diff_header_oldfile } else { break } } + diff_header_oldfile { if {[regexp {^---} $l]} {set state diff_header_newfile} else { break } } + diff_header_newfile { + if {[regexp {^\+\+\+ b/(.*)$} $l _ fname_match]} { + set fname $fname_match + set state diff_hunk_header + } else { + break + } + } + diff_hunk_header { + if {[regexp {^@@@*.* \+([0-9]+),[0-9]+ @@@*$} $l _ lnum_matched]} { + set lnum $lnum_matched + break + } + } + } + } + + if {$id eq {}} { + error_popup [mc "Parsing output of git log failed"] + return 0 + } + + if {![commitinview $id $curview]} { + error_popup [mc "The latest change is in commit %s, \ + which is not in this view" [shortids $id]] + return 0 + } + + selectline [rowofcommit $id] 1 [list $fname $lnum] +} + +proc prepare_show_selection_source {} { + global ctext + global selection_source_data + + set selection [$ctext tag ranges sel] + if {[llength $selection] != 2} { + set selection_source_data {} + return + } + set start_line [lindex [split [lindex $selection 0] "."] 0] + set end_index [split [lindex $selection 1] "."] + if {[lindex $end_index 1] eq 0} { + set end_line [expr [lindex $end_index 0] - 1] + } else { + set end_line [lindex $end_index 0] + } + set selection_source_data [prepare_show_region_source $start_line $end_line] +} + +proc prepare_show_region_source {start_line end_line} { + global cmitmode + global currentid parents curview + global nullid + if {$start_line > $end_line} { + error_popup "Cannot blame region: $start_line > $end_line" + return {} + } + set f [find_ctext_fileinfo $start_line] + if {$f eq {}} { + return {} + } + if {$currentid eq $nullid} { + return {} + } + set fname [lindex $f 0] + set filebase [lindex $f 1] + if {$cmitmode eq "tree"} { + set id $currentid + set start_lnum [expr $start_line - $filebase] + set end_lnum [expr $end_line - $filebase] + } else { + set id [lindex $parents($curview,$currentid) 0] + set start_lnum {} + set end_lnum {} + foreach cl_spec [resolve_hunk_lines $filebase $start_line $end_line] { + set diffline [lindex $cl_spec 0] + if {$diffline > $end_line} { + break + } elseif {$diffline >= $start_line} { + foreach branch [lindex $cl_spec 1] { + if {[lindex $branch 0] == 1} { + if {$start_lnum eq {}} { + set start_lnum [lindex $branch 1] + } + set end_lnum [lindex $branch 1] + } + } + } + } + } + if {$start_lnum ne {} && $end_lnum ne {}} { + return [list $id $fname $start_lnum [expr $end_lnum - $start_lnum + 1]] + } else { + return {} + } +} + # delete $dir when we see eof on $f (presumably because the child has exited) proc delete_at_eof {f dir} { while {[gets $f line] >= 0} {} -- 1.8.5.2.421.g4cdf8d0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-06-24 18:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-03 20:53 [PATCH] gitk: use single blamestuff for all show_line_source{} calls Max Kirillov 2014-02-03 22:34 ` [PATCH 0/3] gitk: show latest change to region Max Kirillov 2014-02-03 22:41 ` [PATCH 1/3] gitk: refactor: separate generic hunk parsing out of find_hunk_blamespecs{} Max Kirillov 2014-02-03 23:20 ` Eric Sunshine 2014-06-24 18:27 ` Max Kirillov 2014-02-03 22:42 ` [PATCH 2/3] gitk: refactor: separate io from logic in the searching origin of line Max Kirillov 2014-02-03 22:42 ` [PATCH 3/3] gitk: pick selection for region blame Max Kirillov 2014-02-03 22:48 ` [PATCH 3/3 v2] gitk: show latest change to region Max Kirillov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).